validate runner secrets before projection

Author: mirkoSekulic

src/runner-org-sync/internal/k8sstate/k8sstate.go

@@ -35,6 +35,16 @@ const (
 	SecretTokenKey = "token"
 )
 
+// RegistrationSecretState describes whether a per-org runner registration
+// Secret is safe for the ConfigMap to reference.
+type RegistrationSecretState string
+
+const (
+	RegistrationSecretMissing RegistrationSecretState = "missing"
+	RegistrationSecretValid   RegistrationSecretState = "valid"
+	RegistrationSecretInvalid RegistrationSecretState = "invalid"
+)
+
 // Store is the package's only entry point for cluster I/O.
 type Store struct {
 	client    kubernetes.Interface
@@ -78,6 +88,30 @@ func (s *Store) SecretExists(ctx context.Context, name string) (bool, error) {
 	return false, fmt.Errorf("k8sstate: get secret %s: %w", name, err)
 }
 
+// RegistrationSecretStatus reports whether the named Secret exists and has
+// the ownership labels and token data expected for the given org.
+func (s *Store) RegistrationSecretStatus(ctx context.Context, name, org string) (RegistrationSecretState, error) {
+	sec, err := s.client.CoreV1().Secrets(s.namespace).Get(ctx, name, metav1.GetOptions{})
+	if apierrors.IsNotFound(err) {
+		return RegistrationSecretMissing, nil
+	}
+	if err != nil {
+		return "", fmt.Errorf("k8sstate: get registration secret %s: %w", name, err)
+	}
+	if sec.Type != "" && sec.Type != corev1.SecretTypeOpaque {
+		return RegistrationSecretInvalid, nil
+	}
+	if sec.Labels[LabelManagedBy] != ManagedBy ||
+		sec.Labels[LabelComponent] != ComponentRegToken ||
+		sec.Labels[LabelOrg] != org {
+		return RegistrationSecretInvalid, nil
+	}
+	if len(sec.Data[SecretTokenKey]) == 0 {
+		return RegistrationSecretInvalid, nil
+	}
+	return RegistrationSecretValid, nil
+}
+
 // CreateRegistrationSecret creates an Opaque Secret carrying the
 // registration token at key "token", labelled with ManagedBy / Component /
 // Org. Returns the underlying error verbatim so callers can use apierrors.IsAlreadyExists.

src/runner-org-sync/internal/k8sstate/k8sstate_test.go

@@ -68,6 +68,72 @@ func TestSecretExists(t *testing.T) {
 	}
 }
 
+func TestRegistrationSecretStatus(t *testing.T) {
+	c := fake.NewSimpleClientset(
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "valid",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					LabelManagedBy: ManagedBy,
+					LabelComponent: ComponentRegToken,
+					LabelOrg:       "ttd",
+				},
+			},
+			Type: corev1.SecretTypeOpaque,
+			Data: map[string][]byte{SecretTokenKey: []byte("tok")},
+		},
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "foreign",
+				Namespace: testNamespace,
+				Labels:    map[string]string{LabelManagedBy: "someone-else"},
+			},
+			Type: corev1.SecretTypeOpaque,
+			Data: map[string][]byte{SecretTokenKey: []byte("tok")},
+		},
+		&corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "empty-token",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					LabelManagedBy: ManagedBy,
+					LabelComponent: ComponentRegToken,
+					LabelOrg:       "ttd",
+				},
+			},
+			Type: corev1.SecretTypeOpaque,
+			Data: map[string][]byte{SecretTokenKey: nil},
+		},
+	)
+	s := NewStore(c, testNamespace)
+
+	tests := []struct {
+		name       string
+		secretName string
+		org        string
+		want       RegistrationSecretState
+	}{
+		{name: "valid", secretName: "valid", org: "ttd", want: RegistrationSecretValid},
+		{name: "missing", secretName: "missing", org: "ttd", want: RegistrationSecretMissing},
+		{name: "foreign same name", secretName: "foreign", org: "ttd", want: RegistrationSecretInvalid},
+		{name: "wrong org", secretName: "valid", org: "brg", want: RegistrationSecretInvalid},
+		{name: "empty token", secretName: "empty-token", org: "ttd", want: RegistrationSecretInvalid},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := s.RegistrationSecretStatus(context.Background(), tt.secretName, tt.org)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != tt.want {
+				t.Errorf("RegistrationSecretStatus() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestDeleteSecret_IdempotentOnMissing(t *testing.T) {
 	c := fake.NewSimpleClientset()
 	s := NewStore(c, testNamespace)

src/runner-org-sync/internal/reconcile/reconcile.go

@@ -13,12 +13,12 @@ import (
 	"errors"
 	"fmt"
 	"sort"
-	"strings"
 
 	"altinn.studio/runner-org-sync/internal/cdn"
 	"altinn.studio/runner-org-sync/internal/gitea"
 	"altinn.studio/runner-org-sync/internal/k8sstate"
 	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/yaml"
 )
 
 // Defaults used when the caller does not override.
@@ -30,9 +30,10 @@ const (
 
 // Failure stages, surfaced on Report.FailedOrgs[*].Stage.
 const (
-	StageMint   = "mint"
-	StageCreate = "create"
-	StageDelete = "delete"
+	StageValidate = "validate"
+	StageMint     = "mint"
+	StageCreate   = "create"
+	StageDelete   = "delete"
 )
 
 // OrgSource produces the discovered org population (typically the CDN client).
@@ -50,7 +51,7 @@ type TokenMinter interface {
 // SecretStore is the cluster I/O surface the Reconciler needs.
 type SecretStore interface {
 	ListManagedSecrets(ctx context.Context) ([]corev1.Secret, error)
-	SecretExists(ctx context.Context, name string) (bool, error)
+	RegistrationSecretStatus(ctx context.Context, name, org string) (k8sstate.RegistrationSecretState, error)
 	CreateRegistrationSecret(ctx context.Context, name, org, token string) error
 	DeleteSecret(ctx context.Context, name string) error
 	ApplyConfigMap(ctx context.Context, name string, data map[string]string) (bool, error)
@@ -171,16 +172,24 @@ func (r *Reconciler) Run(ctx context.Context) (Report, error) {
 	orgHasSecret := make(map[string]bool, len(desired))
 	for _, org := range desired {
 		name := r.secretNameFor(org.Code)
-		exists, err := r.store.SecretExists(ctx, name)
+		status, err := r.store.RegistrationSecretStatus(ctx, name, org.Code)
 		if err != nil {
-			// SecretExists hitting a transient apiserver error is fatal for
+			// The lookup hitting a transient apiserver error is fatal for
 			// this run — without this lookup we cannot decide mint-or-skip.
-			return report, fmt.Errorf("reconcile: check secret %s: %w", name, err)
+			return report, fmt.Errorf("reconcile: check registration secret %s: %w", name, err)
 		}
-		if exists {
+		switch status {
+		case k8sstate.RegistrationSecretValid:
 			report.SecretsSkipped = append(report.SecretsSkipped, org.Code)
 			orgHasSecret[org.Code] = true
 			continue
+		case k8sstate.RegistrationSecretInvalid:
+			report.FailedOrgs = append(report.FailedOrgs, OrgFailure{
+				Org:   org.Code,
+				Stage: StageValidate,
+				Err:   fmt.Errorf("registration secret %s exists but is not a valid runner token secret", name),
+			})
+			continue
 		}
 		token, err := r.minter.MintRegistrationToken(ctx, org.Code)
 		if err != nil {
@@ -287,15 +296,26 @@ func (r *Reconciler) filter(orgs []cdn.Org, report *Report) []cdn.Org {
 // on the consumer side, so a runner-org-sync-supplied replicas field would
 // be ignored at best and misleading at worst.
 func renderRunners(orgs []string, secretNameFor func(org string) string) string {
-	if len(orgs) == 0 {
-		return "[]\n"
-	}
-	var b strings.Builder
+	runners := make([]runnerConfig, 0, len(orgs))
 	for _, org := range orgs {
-		fmt.Fprintf(&b, "- name: %s\n", org)
-		fmt.Fprintf(&b, "  registrationTokenSecretName: %s\n", secretNameFor(org))
+		runners = append(runners, runnerConfig{
+			Name:                        org,
+			RegistrationTokenSecretName: secretNameFor(org),
+		})
+	}
+	out, err := yaml.Marshal(runners)
+	if err != nil {
+		// The input is a simple slice of strings rendered into a static struct;
+		// yaml.Marshal should not fail. Keep the historical empty-list output
+		// if it ever does, so the chart does not reference stale runners.
+		return "[]\n"
 	}
-	return b.String()
+	return string(out)
+}
+
+type runnerConfig struct {
+	Name                        string `json:"name"`
+	RegistrationTokenSecretName string `json:"registrationTokenSecretName"`
 }
 
 func orgCodes(orgs []cdn.Org) []string {

src/runner-org-sync/internal/reconcile/reconcile_test.go

@@ -10,6 +10,7 @@ import (
 
 	"altinn.studio/runner-org-sync/internal/cdn"
 	"altinn.studio/runner-org-sync/internal/gitea"
+	"altinn.studio/runner-org-sync/internal/k8sstate"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -43,7 +44,7 @@ func (m *stubMinter) MintRegistrationToken(_ context.Context, org string) (strin
 
 type stubStore struct {
 	managed         []corev1.Secret
-	existsByName    map[string]bool
+	statusByName    map[string]k8sstate.RegistrationSecretState
 	createErr       map[string]error
 	deleteErr       map[string]error
 	applyCMErr      error
@@ -58,7 +59,7 @@ type stubStore struct {
 
 func newStubStore() *stubStore {
 	return &stubStore{
-		existsByName:    map[string]bool{},
+		statusByName:    map[string]k8sstate.RegistrationSecretState{},
 		createErr:       map[string]error{},
 		deleteErr:       map[string]error{},
 		createdOrgs:     map[string]string{},
@@ -70,11 +71,14 @@ func (s *stubStore) ListManagedSecrets(_ context.Context) ([]corev1.Secret, erro
 	return s.managed, s.listErr
 }
 
-func (s *stubStore) SecretExists(_ context.Context, name string) (bool, error) {
+func (s *stubStore) RegistrationSecretStatus(_ context.Context, name, _ string) (k8sstate.RegistrationSecretState, error) {
 	if s.existsErr != nil {
-		return false, s.existsErr
+		return "", s.existsErr
 	}
-	return s.existsByName[name], nil
+	if status, ok := s.statusByName[name]; ok {
+		return status, nil
+	}
+	return k8sstate.RegistrationSecretMissing, nil
 }
 
 func (s *stubStore) CreateRegistrationSecret(_ context.Context, name, org, _ string) error {
@@ -83,7 +87,7 @@ func (s *stubStore) CreateRegistrationSecret(_ context.Context, name, org, _ str
 	}
 	s.createdSecrets = append(s.createdSecrets, name)
 	s.createdOrgs[name] = org
-	s.existsByName[name] = true
+	s.statusByName[name] = k8sstate.RegistrationSecretValid
 	return nil
 }
 
@@ -190,8 +194,8 @@ func TestRun_IdempotentReRun(t *testing.T) {
 	minter := &stubMinter{}
 	store := newStubStore()
 	// pre-populate existing state — secrets exist for both orgs and we own them.
-	store.existsByName["altinn-gitea-runner-ttd-secret"] = true
-	store.existsByName["altinn-gitea-runner-brg-secret"] = true
+	store.statusByName["altinn-gitea-runner-ttd-secret"] = k8sstate.RegistrationSecretValid
+	store.statusByName["altinn-gitea-runner-brg-secret"] = k8sstate.RegistrationSecretValid
 	store.managed = []corev1.Secret{
 		managedSecret("altinn-gitea-runner-ttd-secret", "ttd"),
 		managedSecret("altinn-gitea-runner-brg-secret", "brg"),
@@ -226,8 +230,8 @@ func TestRun_OrgAdded(t *testing.T) {
 	}}
 	minter := &stubMinter{}
 	store := newStubStore()
-	store.existsByName["altinn-gitea-runner-ttd-secret"] = true
-	store.existsByName["altinn-gitea-runner-brg-secret"] = true
+	store.statusByName["altinn-gitea-runner-ttd-secret"] = k8sstate.RegistrationSecretValid
+	store.statusByName["altinn-gitea-runner-brg-secret"] = k8sstate.RegistrationSecretValid
 	store.managed = []corev1.Secret{
 		managedSecret("altinn-gitea-runner-ttd-secret", "ttd"),
 		managedSecret("altinn-gitea-runner-brg-secret", "brg"),
@@ -254,8 +258,8 @@ func TestRun_OrgRemoved(t *testing.T) {
 	}}
 	minter := &stubMinter{}
 	store := newStubStore()
-	store.existsByName["altinn-gitea-runner-ttd-secret"] = true
-	store.existsByName["altinn-gitea-runner-brg-secret"] = true
+	store.statusByName["altinn-gitea-runner-ttd-secret"] = k8sstate.RegistrationSecretValid
+	store.statusByName["altinn-gitea-runner-brg-secret"] = k8sstate.RegistrationSecretValid
 	store.managed = []corev1.Secret{
 		managedSecret("altinn-gitea-runner-ttd-secret", "ttd"),
 		managedSecret("altinn-gitea-runner-brg-secret", "brg"),
@@ -347,6 +351,30 @@ func TestRun_GiteaPartialFailure(t *testing.T) {
 	}
 }
 
+func TestRun_InvalidExistingSecretIsNotProjected(t *testing.T) {
+	src := &stubSource{orgs: []cdn.Org{
+		{Code: "ttd", Environments: []string{"tt02"}},
+	}}
+	minter := &stubMinter{}
+	store := newStubStore()
+	store.statusByName["altinn-gitea-runner-ttd-secret"] = k8sstate.RegistrationSecretInvalid
+
+	rep := runReconciler(t, src, minter, store, []string{"ttd"}, false)
+
+	if rep.Outcome != OutcomePartial {
+		t.Errorf("outcome = %v, want partial", rep.Outcome)
+	}
+	if len(rep.FailedOrgs) != 1 || rep.FailedOrgs[0].Org != "ttd" || rep.FailedOrgs[0].Stage != StageValidate {
+		t.Errorf("FailedOrgs = %v, want [{ttd validate ...}]", rep.FailedOrgs)
+	}
+	if len(minter.calls) != 0 {
+		t.Errorf("minter should not be called when same-name invalid secret exists; got %v", minter.calls)
+	}
+	if got := store.appliedCMData[ConfigMapDataKey]; got != "[]\n" {
+		t.Errorf("ConfigMap body = %q, want empty runner list", got)
+	}
+}
+
 // --- additional coverage ----------------------------------------------------
 
 // Auth failures hit every org with the same PAT — failing fast avoids a
@@ -449,7 +477,7 @@ func TestRun_SyncAllSkipsWhitelist(t *testing.T) {
 func TestRun_UnlabelledManagedSecretIsSkippedOnDelete(t *testing.T) {
 	src := &stubSource{orgs: []cdn.Org{{Code: "ttd", Environments: []string{"tt02"}}}}
 	store := newStubStore()
-	store.existsByName["altinn-gitea-runner-ttd-secret"] = true
+	store.statusByName["altinn-gitea-runner-ttd-secret"] = k8sstate.RegistrationSecretValid
 	store.managed = []corev1.Secret{
 		managedSecret("altinn-gitea-runner-ttd-secret", "ttd"),
 		// drift: managed-by label but no org label