Stage 2: TerdutTeam (create, mint team credential, rename/oidc-groups, delete)
CI / test (push) Successful in 1m41s
CI / test (push) Successful in 1m41s
Implements ROADMAP.md Stage 2 against terdut-server's now-real
GET /api/teams?name= (TEAM-LOOKUP.md, landed in terdut-server just
before this commit) -- without it, the adopt-on-409 pattern this
controller depends on for team creation had no server-side lookup to
call, the same gap TerdutServer's own bootstrap flow hit and fixed in
Stage 1.
- api/v1alpha1: TerdutTeamSpec per DESIGN.md §4.2 (serverRef, displayName,
oidc). status.credentialsSecretRef drops namespace for key, matching
the fix already applied to TerdutServer's.
- internal/controller:
- terdutteam_controller.go: resolves serverRef (same-namespace by
default; cross-namespace gated by the target TerdutServer's
spec.allowedTeams, DESIGN.md §4.6), waits for that TerdutServer to be
Bootstrapped (no cross-controller RPC -- reads its
status.credentialsSecretRef directly, DESIGN.md §5), creates the team
and mints its team-scoped credential using the TerdutServer's
instance-scoped one, then applies rename/oidc-groups with the
team-scoped credential every reconcile (both are idempotent PUTs of
the whole resource -- applied unconditionally rather than diffed
against a stored last-applied value, same "cheap because it's small"
reasoning §5 already gives the escalation policy's whole-policy PUT).
- terdutteam_allowedteams.go: the §4.6 consent check in isolation from
any client, unit-tested directly against hand-built inputs.
- terdutteam_bootstrap.go: create-or-adopt-on-409 for the team itself
(via TEAM-LOOKUP.md) and for its team-scoped service account (via the
same GET-by-name+mint-new-key pattern Stage 1 already uses for the
instance account).
- secrets.go: extracted TerdutServer's write/read-credential-Secret
helpers into free functions, now shared by both controllers rather
than duplicated.
- Finalizer deletes the team server-side (owner-gated, needs the
team-scoped credential -- confirmed against source that an
instance-scoped one does not satisfy requireTeamOwner, same finding
as TEAM-LOOKUP.md's) and cleans up its credentials Secret. A team
created but never fully reconciled to Ready (no team-scoped
credential ever minted) is left orphaned server-side on delete -- a
known, documented limitation (terdut-server has no delete path that
doesn't require owner-equivalent access), not a silent gap.
- internal/tdclient: Team type, CreateTeam, GetTeamByName, RenameTeam,
DeleteTeam, SetTeamOIDCGroups, CreateTeamServiceAccount -- matching
terdut-server's real handlers' shapes field-for-field, same as Stage
1's client additions.
- Tests: envtest covering the happy path, both not-ready reasons
(ServerRefNotFound, WaitingForServer), cross-namespace allow/deny
(default-closed and explicit All), both adopt-on-409 paths (team
itself, team-scoped service account), and deletion. Extended the shared
fakeTerdutServer (Stage 1) with team endpoints rather than writing a
second, separately-drifting fake. 72.8%/30.6% coverage, 0 lint issues.
Verified locally: make fmt lint test build all clean.
This commit is contained in:
@@ -23,6 +23,14 @@ import (
|
||||
"git.ryuvia.com/niklas/terdut-operator/internal/tdclient"
|
||||
)
|
||||
|
||||
// fakeVersionString/errJSONKey are shared by every response fakeTerdutServer
|
||||
// writes -- goconst would otherwise flag "test" and "error" as repeated
|
||||
// literals across its handlers.
|
||||
const (
|
||||
fakeVersionString = "test"
|
||||
errJSONKey = "error"
|
||||
)
|
||||
|
||||
// fakeTerdutServer reproduces the exact stateful semantics of
|
||||
// /api/bootstrap, /api/service-accounts and /api/version that the
|
||||
// bootstrap flow depends on (DESIGN.md §6, §11: "terdut-server's REST API
|
||||
@@ -37,10 +45,23 @@ type fakeTerdutServer struct {
|
||||
nextID int64
|
||||
accounts map[string]int64 // name -> id
|
||||
keyMints map[int64]int // id -> number of keys minted so far
|
||||
|
||||
nextTeamID int64
|
||||
teams map[string]int64 // name -> id
|
||||
teamNames map[int64]string // id -> current name (renames update this)
|
||||
teamOIDC map[int64][2]string
|
||||
teamDelete map[int64]bool // id -> true once DELETEd, for 404-on-redelete
|
||||
}
|
||||
|
||||
func newFakeTerdutServer() (*fakeTerdutServer, *httptest.Server) {
|
||||
f := &fakeTerdutServer{accounts: map[string]int64{}, keyMints: map[int64]int{}}
|
||||
f := &fakeTerdutServer{
|
||||
accounts: map[string]int64{},
|
||||
keyMints: map[int64]int{},
|
||||
teams: map[string]int64{},
|
||||
teamNames: map[int64]string{},
|
||||
teamOIDC: map[int64][2]string{},
|
||||
teamDelete: map[int64]bool{},
|
||||
}
|
||||
return f, httptest.NewServer(f)
|
||||
}
|
||||
|
||||
@@ -50,11 +71,11 @@ func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
switch {
|
||||
case r.URL.Path == "/api/version":
|
||||
writeJSON(w, http.StatusOK, map[string]string{"version": "test"})
|
||||
writeJSON(w, http.StatusOK, map[string]string{"version": fakeVersionString})
|
||||
|
||||
case r.URL.Path == "/api/bootstrap" && r.Method == http.MethodPost:
|
||||
if f.bootstrapped || f.bootstrap403 {
|
||||
writeJSON(w, http.StatusForbidden, map[string]string{"error": "bootstrap already completed"})
|
||||
writeJSON(w, http.StatusForbidden, map[string]string{errJSONKey: "bootstrap already completed"})
|
||||
return
|
||||
}
|
||||
f.bootstrapped = true
|
||||
@@ -69,7 +90,7 @@ func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
if _, exists := f.accounts[req.Name]; exists {
|
||||
writeJSON(w, http.StatusConflict, map[string]string{"error": "a service account with that name already exists"})
|
||||
writeJSON(w, http.StatusConflict, map[string]string{errJSONKey: "a service account with that name already exists"})
|
||||
return
|
||||
}
|
||||
f.nextID++
|
||||
@@ -90,6 +111,32 @@ func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
writeJSON(w, http.StatusOK, []tdclient.ServiceAccount{{ID: id, Name: name, Scope: "instance"}})
|
||||
|
||||
case r.URL.Path == "/api/teams" && r.Method == http.MethodPost:
|
||||
var req struct {
|
||||
Name string `json:"name"`
|
||||
}
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
if _, exists := f.teams[req.Name]; exists {
|
||||
writeJSON(w, http.StatusConflict, map[string]string{errJSONKey: "a team with that name already exists"})
|
||||
return
|
||||
}
|
||||
f.nextTeamID++
|
||||
id := f.nextTeamID
|
||||
f.teams[req.Name] = id
|
||||
f.teamNames[id] = req.Name
|
||||
writeJSON(w, http.StatusCreated, tdclient.Team{ID: id, Name: req.Name})
|
||||
|
||||
case r.URL.Path == "/api/teams" && r.Method == http.MethodGet:
|
||||
// The controller never calls this without ?name= (TEAM-LOOKUP.md's
|
||||
// own lookup shape) -- the fake only needs to answer that form.
|
||||
name := r.URL.Query().Get("name")
|
||||
id, exists := f.teams[name]
|
||||
if !exists {
|
||||
writeJSON(w, http.StatusOK, []tdclient.Team{})
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, []tdclient.Team{{ID: id, Name: f.teamNames[id]}})
|
||||
|
||||
default:
|
||||
if id, name, ok := parseKeysPath(r.URL.Path); ok && r.Method == http.MethodPost {
|
||||
f.keyMints[id]++
|
||||
@@ -99,10 +146,78 @@ func (f *fakeTerdutServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
})
|
||||
return
|
||||
}
|
||||
if id, ok := parseTeamPath(r.URL.Path); ok {
|
||||
f.handleTeamByID(w, r, id)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}
|
||||
|
||||
// handleTeamByID answers PUT /api/teams/{id}, PUT /api/teams/{id}/oidc-groups
|
||||
// and DELETE /api/teams/{id}.
|
||||
func (f *fakeTerdutServer) handleTeamByID(w http.ResponseWriter, r *http.Request, id int64) {
|
||||
oidcSuffix := fmt.Sprintf("/api/teams/%d/oidc-groups", id)
|
||||
|
||||
switch {
|
||||
case r.URL.Path == oidcSuffix && r.Method == http.MethodPut:
|
||||
var req struct {
|
||||
MemberGroup string `json:"member_group"`
|
||||
OwnerGroup string `json:"owner_group"`
|
||||
}
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
if _, exists := f.teamNames[id]; !exists {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
f.teamOIDC[id] = [2]string{req.MemberGroup, req.OwnerGroup}
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
|
||||
case r.URL.Path == fmt.Sprintf("/api/teams/%d", id) && r.Method == http.MethodPut:
|
||||
var req struct {
|
||||
Name string `json:"name"`
|
||||
}
|
||||
_ = json.NewDecoder(r.Body).Decode(&req)
|
||||
oldName, exists := f.teamNames[id]
|
||||
if !exists {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
delete(f.teams, oldName)
|
||||
f.teamNames[id] = req.Name
|
||||
f.teams[req.Name] = id
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
|
||||
case r.URL.Path == fmt.Sprintf("/api/teams/%d", id) && r.Method == http.MethodDelete:
|
||||
name, exists := f.teamNames[id]
|
||||
if !exists {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
delete(f.teams, name)
|
||||
delete(f.teamNames, id)
|
||||
f.teamDelete[id] = true
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}
|
||||
|
||||
// parseTeamPath extracts the numeric id from "/api/teams/{id}" or
|
||||
// "/api/teams/{id}/oidc-groups" -- anything with more or fewer segments
|
||||
// doesn't match (handleTeamByID's own switch sorts out which of the two).
|
||||
func parseTeamPath(path string) (id int64, ok bool) {
|
||||
var parsedID int64
|
||||
if n, err := fmt.Sscanf(path, "/api/teams/%d/oidc-groups", &parsedID); err == nil && n == 1 {
|
||||
return parsedID, true
|
||||
}
|
||||
if n, err := fmt.Sscanf(path, "/api/teams/%d", &parsedID); err == nil && n == 1 {
|
||||
return parsedID, true
|
||||
}
|
||||
return 0, false
|
||||
}
|
||||
|
||||
func parseKeysPath(path string) (id int64, mintName string, ok bool) {
|
||||
var parsedID int64
|
||||
n, err := fmt.Sscanf(path, "/api/service-accounts/%d/keys", &parsedID)
|
||||
@@ -151,9 +266,9 @@ var _ = Describe("TerdutServer Controller", func() {
|
||||
|
||||
dsnSpec := func() terdutv1alpha1.TerdutServerSpec {
|
||||
return terdutv1alpha1.TerdutServerSpec{
|
||||
Image: terdutv1alpha1.ImageSpec{Repository: "example.invalid/terdut-server", Tag: "test"},
|
||||
Image: terdutv1alpha1.ImageSpec{Repository: testImageRepo, Tag: testImageTag},
|
||||
Networking: terdutv1alpha1.NetworkingSpec{Hostname: "terdut.example.invalid", ServicePort: 8080},
|
||||
Database: terdutv1alpha1.DatabaseSpec{DSN: "postgres://terdut@test-postgres:5432/terdut?sslmode=require"},
|
||||
Database: terdutv1alpha1.DatabaseSpec{DSN: testDSN},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -212,7 +327,7 @@ var _ = Describe("TerdutServer Controller", func() {
|
||||
for _, e := range deploy.Spec.Template.Spec.Containers[0].Env {
|
||||
envNames[e.Name] = e.Value
|
||||
}
|
||||
Expect(envNames).To(HaveKeyWithValue("TERDUT_DB_DSN", "postgres://terdut@test-postgres:5432/terdut?sslmode=require"))
|
||||
Expect(envNames).To(HaveKeyWithValue("TERDUT_DB_DSN", testDSN))
|
||||
Expect(envNames).To(HaveKeyWithValue("TERDUT_OPERATOR_MODE", "true"))
|
||||
|
||||
var svc corev1.Service
|
||||
@@ -266,7 +381,7 @@ var _ = Describe("TerdutServer Controller", func() {
|
||||
|
||||
// The earlier attempt's checkpoint survived (that's how this
|
||||
// reconcile can authenticate at all to recover).
|
||||
Expect(reconciler.writeSecret(ctx, checkpointSecretName(&terdutv1alpha1.TerdutServer{
|
||||
Expect(writeOperatorSecret(ctx, k8sClient, operatorNamespace, checkpointSecretName(&terdutv1alpha1.TerdutServer{
|
||||
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: operatorNamespace},
|
||||
}), "admin-key-raw")).To(Succeed())
|
||||
|
||||
|
||||
Reference in New Issue
Block a user