diff --git a/cmd/main.go b/cmd/main.go index 0a16b97..932d9aa 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -25,6 +25,7 @@ import ( "strconv" "strings" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/nats" "sigs.k8s.io/controller-runtime/pkg/cache" @@ -265,7 +266,7 @@ func main() { namespace = string(controllerNamespace) } - accountConfig, err := account.NewConfig(operatorNatsCluster, namespace, defaultNatsURL) + accountConfig, err := account.NewConfig(operatorNatsCluster, domain.Namespace(namespace), defaultNatsURL) if err != nil { setupLog.Error(err, "invalid configuration") os.Exit(1) diff --git a/internal/account/account.go b/internal/account/account.go index 8360c36..e079288 100644 --- a/internal/account/account.go +++ b/internal/account/account.go @@ -7,6 +7,7 @@ import ( "github.com/WirelessCar/nauth/api/v1alpha1" "github.com/WirelessCar/nauth/internal/controller" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/jwt/v2" @@ -75,6 +76,11 @@ func (a *Manager) validate() error { } func (a *Manager) Create(ctx context.Context, state *v1alpha1.Account) (*controller.AccountResult, error) { + accountRef := domain.NewNamespacedName(state.GetNamespace(), state.GetName()) + if err := accountRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + var accountPublicKey string var accountSigningPublicKey string var accountKeyPair nkeys.KeyPair @@ -84,7 +90,7 @@ func (a *Manager) Create(ctx context.Context, state *v1alpha1.Account) (*control if err != nil { return nil, fmt.Errorf("failed to resolve cluster target: %w", err) } - accountSecrets, err := a.secretManager.GetSecrets(ctx, state.GetNamespace(), state.GetName(), "") + accountSecrets, err := a.secretManager.GetSecrets(ctx, accountRef, "") if err == nil { accountKeyPair = accountSecrets.Root accountPublicKey, err = accountKeyPair.PublicKey() @@ -117,12 +123,12 @@ func (a *Manager) Create(ctx context.Context, state *v1alpha1.Account) (*control } } - err = a.secretManager.ApplyRootSecret(ctx, state.GetNamespace(), state.GetName(), accountKeyPair) + err = a.secretManager.ApplyRootSecret(ctx, accountRef, accountKeyPair) if err != nil { return nil, fmt.Errorf("failed to apply account root secret during creation: %w", err) } - err = a.secretManager.ApplySignSecret(ctx, state.GetNamespace(), state.GetName(), accountPublicKey, accountSigningKeyPair) + err = a.secretManager.ApplySignSecret(ctx, accountRef, accountPublicKey, accountSigningKeyPair) if err != nil { return nil, fmt.Errorf("failed to apply account signing secret during creation: %w", err) } @@ -166,13 +172,18 @@ func (a *Manager) Create(ctx context.Context, state *v1alpha1.Account) (*control } func (a *Manager) Update(ctx context.Context, state *v1alpha1.Account) (*controller.AccountResult, error) { + accountRef := domain.NewNamespacedName(state.GetNamespace(), state.GetName()) + if err := accountRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + cluster, err := a.resolveClusterTarget(ctx, state) if err != nil { return nil, fmt.Errorf("failed to resolve cluster target: %w", err) } accountID := state.GetLabels()[k8s.LabelAccountID] - secrets, err := a.secretManager.GetSecrets(ctx, state.GetNamespace(), state.GetName(), accountID) + secrets, err := a.secretManager.GetSecrets(ctx, accountRef, accountID) if err != nil { return nil, err } @@ -232,6 +243,11 @@ func (a *Manager) Update(ctx context.Context, state *v1alpha1.Account) (*control } func (a *Manager) Import(ctx context.Context, state *v1alpha1.Account) (*controller.AccountResult, error) { + accountRef := domain.NewNamespacedName(state.GetNamespace(), state.GetName()) + if err := accountRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + cluster, err := a.resolveClusterTarget(ctx, state) if err != nil { return nil, fmt.Errorf("failed to resolve cluster target: %w", err) @@ -247,7 +263,7 @@ func (a *Manager) Import(ctx context.Context, state *v1alpha1.Account) (*control return nil, fmt.Errorf("account ID is missing for account %s during import", state.GetName()) } - secrets, err := a.secretManager.GetSecrets(ctx, state.GetNamespace(), state.GetName(), accountID) + secrets, err := a.secretManager.GetSecrets(ctx, accountRef, accountID) if err != nil { return nil, fmt.Errorf("failed to get secrets for account %s during import: %w", accountID, err) } @@ -287,6 +303,11 @@ func (a *Manager) Import(ctx context.Context, state *v1alpha1.Account) (*control } func (a *Manager) Delete(ctx context.Context, state *v1alpha1.Account) error { + accountRef := domain.NewNamespacedName(state.GetNamespace(), state.GetName()) + if err := accountRef.Validate(); err != nil { + return fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + cluster, err := a.resolveClusterTarget(ctx, state) if err != nil { return fmt.Errorf("failed to resolve cluster target: %w", err) @@ -320,7 +341,7 @@ func (a *Manager) Delete(ctx context.Context, state *v1alpha1.Account) error { return fmt.Errorf("failed to delete account: %w", err) } - err = a.secretManager.DeleteAll(ctx, state.GetNamespace(), state.GetName(), accountID) + err = a.secretManager.DeleteAll(ctx, accountRef, accountID) if err != nil { return fmt.Errorf("failed to delete account secrets: %w", err) } diff --git a/internal/account/account_test.go b/internal/account/account_test.go index a098a7a..71ab3c6 100644 --- a/internal/account/account_test.go +++ b/internal/account/account_test.go @@ -7,6 +7,7 @@ import ( "github.com/WirelessCar/nauth/api/v1alpha1" "github.com/WirelessCar/nauth/internal/controller" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/jwt/v2" @@ -87,14 +88,15 @@ func (t *ManagerTestSuite) Test_Create_ShouldSucceed() { caughtSignAccountID string caughtSignKeyPair nkeys.KeyPair ) + accountRef := domain.NewNamespacedName("account-namespace", "account-name") var natsLimitsSubs int64 = 100 t.clusterTargetResolverMock.mockGetClusterTarget(t.ctx, nil, &t.clusterTarget) - t.secretManagerMock.mockGetSecretsError(t.ctx, "account-namespace", "account-name", "", fmt.Errorf("no secrets found")) - t.secretManagerMock.mockApplyRootSecretUnknown(t.ctx, "account-namespace", "account-name", func(rootKeyPair nkeys.KeyPair) { + t.secretManagerMock.mockGetSecretsError(t.ctx, accountRef, "", fmt.Errorf("no secrets found")) + t.secretManagerMock.mockApplyRootSecretUnknown(t.ctx, accountRef, func(rootKeyPair nkeys.KeyPair) { caughtRootKeyPair = rootKeyPair }) - t.secretManagerMock.mockApplySignSecretUnknown(t.ctx, "account-namespace", "account-name", func(accountID string, signKeyPair nkeys.KeyPair) { + t.secretManagerMock.mockApplySignSecretUnknown(t.ctx, accountRef, func(accountID string, signKeyPair nkeys.KeyPair) { caughtSignAccountID = accountID caughtSignKeyPair = signKeyPair }) @@ -134,17 +136,18 @@ func (t *ManagerTestSuite) Test_Create_ShouldSucceed_WhenAccountExplicitCluster( caughtSignKeyPair nkeys.KeyPair ) + accountRef := domain.NewNamespacedName("account-namespace", "account-name") natsLimitsSubs := int64(100) t.clusterTargetResolverMock.mockGetClusterTarget(t.ctx, &v1alpha1.NatsClusterRef{ Namespace: "account-namespace", Name: "account-namespace-cluster", }, &t.clusterTarget) - t.secretManagerMock.mockGetSecretsError(t.ctx, "account-namespace", "account-name", "", fmt.Errorf("no secrets found")) - t.secretManagerMock.mockApplyRootSecretUnknown(t.ctx, "account-namespace", "account-name", func(rootKeyPair nkeys.KeyPair) { + t.secretManagerMock.mockGetSecretsError(t.ctx, accountRef, "", fmt.Errorf("no secrets found")) + t.secretManagerMock.mockApplyRootSecretUnknown(t.ctx, accountRef, func(rootKeyPair nkeys.KeyPair) { caughtRootKeyPair = rootKeyPair }) - t.secretManagerMock.mockApplySignSecretUnknown(t.ctx, "account-namespace", "account-name", func(accountID string, signKeyPair nkeys.KeyPair) { + t.secretManagerMock.mockApplySignSecretUnknown(t.ctx, accountRef, func(accountID string, signKeyPair nkeys.KeyPair) { caughtSignAccountID = accountID caughtSignKeyPair = signKeyPair }) @@ -183,18 +186,19 @@ func (t *ManagerTestSuite) Test_Create_ShouldSucceed_WhenSecretsAlreadyExist() { var ( caughtAccountJWT string ) + accountRef := domain.NewNamespacedName("account-namespace", "account-name") accountRootKey, _ := nkeys.CreateAccount() accountID, _ := accountRootKey.PublicKey() accountSignKey, _ := nkeys.CreateAccount() var natsLimitsSubs int64 = 100 t.clusterTargetResolverMock.mockGetClusterTarget(t.ctx, nil, &t.clusterTarget) - t.secretManagerMock.mockGetSecrets(t.ctx, "account-namespace", "account-name", "", &Secrets{ + t.secretManagerMock.mockGetSecrets(t.ctx, accountRef, "", &Secrets{ Root: accountRootKey, Sign: accountSignKey, }) - t.secretManagerMock.mockApplyRootSecret(t.ctx, "account-namespace", "account-name", accountRootKey) - t.secretManagerMock.mockApplySignSecret(t.ctx, "account-namespace", "account-name", accountID, accountSignKey) + t.secretManagerMock.mockApplyRootSecret(t.ctx, accountRef, accountRootKey) + t.secretManagerMock.mockApplySignSecret(t.ctx, accountRef, accountID, accountSignKey) t.natsClientMock.mockConnect(t.natsURL, t.sauCreds, t.natsConnMock) t.natsConnMock.mockUploadAccountJWTCatch(func(jwt string) { caughtAccountJWT = jwt }) t.natsConnMock.mockDisconnect() @@ -244,12 +248,13 @@ func (t *ManagerTestSuite) Test_Update_ShouldSucceed() { var ( caughtAccountJWT string ) + accountRef := domain.NewNamespacedName("account-namespace", "account-name") accountRootKey, _ := nkeys.CreateAccount() accountID, _ := accountRootKey.PublicKey() accountSignKey, _ := nkeys.CreateAccount() t.clusterTargetResolverMock.mockGetClusterTarget(t.ctx, nil, &t.clusterTarget) - t.secretManagerMock.mockGetSecrets(t.ctx, "account-namespace", "account-name", accountID, &Secrets{ + t.secretManagerMock.mockGetSecrets(t.ctx, accountRef, accountID, &Secrets{ Root: accountRootKey, Sign: accountSignKey, }) @@ -278,6 +283,7 @@ func (t *ManagerTestSuite) Test_Update_ShouldSucceed() { func (t *ManagerTestSuite) Test_Import_ShouldSucceed() { // Given + accountRef := domain.NewNamespacedName("account-namespace", "account-name") accountRootKey, _ := nkeys.CreateAccount() accountID, _ := accountRootKey.PublicKey() accountSignKey, _ := nkeys.CreateAccount() @@ -297,7 +303,7 @@ func (t *ManagerTestSuite) Test_Import_ShouldSucceed() { t.NoError(err, "failed to encode existing account JWT") t.clusterTargetResolverMock.mockGetClusterTarget(t.ctx, nil, &t.clusterTarget) - t.secretManagerMock.mockGetSecrets(t.ctx, "account-namespace", "account-name", accountID, &Secrets{ + t.secretManagerMock.mockGetSecrets(t.ctx, accountRef, accountID, &Secrets{ Root: accountRootKey, Sign: accountSignKey, }) @@ -328,6 +334,7 @@ func (t *ManagerTestSuite) Test_Delete_ShouldSucceed() { var ( caughtDeleteJWT string ) + accountRef := domain.NewNamespacedName("account-namespace", "account-name") accountRootKey, _ := nkeys.CreateAccount() accountID, _ := accountRootKey.PublicKey() @@ -335,7 +342,7 @@ func (t *ManagerTestSuite) Test_Delete_ShouldSucceed() { t.natsClientMock.mockConnect(t.natsURL, t.sauCreds, t.natsConnMock) t.natsConnMock.mockDeleteAccountJWTCatch(func(jwt string) { caughtDeleteJWT = jwt }) t.natsConnMock.mockDisconnect() - t.secretManagerMock.mockDeleteAll(t.ctx, "account-namespace", "account-name", accountID) + t.secretManagerMock.mockDeleteAll(t.ctx, accountRef, accountID) // When err := t.unitUnderTest.Delete(t.ctx, &v1alpha1.Account{ @@ -421,67 +428,67 @@ func newSecretManagerMock() *secretManagerMock { return &secretManagerMock{} } -func (m *secretManagerMock) ApplyRootSecret(ctx context.Context, namespace, accountName string, rootKeyPair nkeys.KeyPair) error { - args := m.Called(ctx, namespace, accountName, rootKeyPair) +func (m *secretManagerMock) ApplyRootSecret(ctx context.Context, accountRef domain.NamespacedName, rootKeyPair nkeys.KeyPair) error { + args := m.Called(ctx, accountRef, rootKeyPair) return args.Error(0) } -func (m *secretManagerMock) mockApplyRootSecret(ctx context.Context, namespace, accountName string, rootKeyPair nkeys.KeyPair) { - m.On("ApplyRootSecret", ctx, namespace, accountName, rootKeyPair).Return(nil) +func (m *secretManagerMock) mockApplyRootSecret(ctx context.Context, accountRef domain.NamespacedName, rootKeyPair nkeys.KeyPair) { + m.On("ApplyRootSecret", ctx, accountRef, rootKeyPair).Return(nil) } -func (m *secretManagerMock) mockApplyRootSecretUnknown(ctx context.Context, namespace, accountName string, catch func(rootKeyPair nkeys.KeyPair)) { - m.On("ApplyRootSecret", ctx, namespace, accountName, mock.Anything). +func (m *secretManagerMock) mockApplyRootSecretUnknown(ctx context.Context, accountRef domain.NamespacedName, catch func(rootKeyPair nkeys.KeyPair)) { + m.On("ApplyRootSecret", ctx, accountRef, mock.Anything). Return(nil). Run(func(args mock.Arguments) { if catch != nil { - catch(args.Get(3).(nkeys.KeyPair)) + catch(args.Get(2).(nkeys.KeyPair)) } }) } -func (m *secretManagerMock) ApplySignSecret(ctx context.Context, namespace, accountName, accountID string, signKeyPair nkeys.KeyPair) error { - args := m.Called(ctx, namespace, accountName, accountID, signKeyPair) +func (m *secretManagerMock) ApplySignSecret(ctx context.Context, accountRef domain.NamespacedName, accountID string, signKeyPair nkeys.KeyPair) error { + args := m.Called(ctx, accountRef, accountID, signKeyPair) return args.Error(0) } -func (m *secretManagerMock) mockApplySignSecret(ctx context.Context, namespace, accountName, accountID string, signKeyPair nkeys.KeyPair) { - m.On("ApplySignSecret", ctx, namespace, accountName, accountID, signKeyPair).Return(nil) +func (m *secretManagerMock) mockApplySignSecret(ctx context.Context, accountRef domain.NamespacedName, accountID string, signKeyPair nkeys.KeyPair) { + m.On("ApplySignSecret", ctx, accountRef, accountID, signKeyPair).Return(nil) } -func (m *secretManagerMock) mockApplySignSecretUnknown(ctx context.Context, namespace, accountName string, catch func(accountID string, signKeyPair nkeys.KeyPair)) { - m.On("ApplySignSecret", ctx, namespace, accountName, mock.Anything, mock.Anything). +func (m *secretManagerMock) mockApplySignSecretUnknown(ctx context.Context, accountRef domain.NamespacedName, catch func(accountID string, signKeyPair nkeys.KeyPair)) { + m.On("ApplySignSecret", ctx, accountRef, mock.Anything, mock.Anything). Return(nil). Run(func(args mock.Arguments) { if catch != nil { - catch(args.String(3), args.Get(4).(nkeys.KeyPair)) + catch(args.String(2), args.Get(3).(nkeys.KeyPair)) } }) } -func (m *secretManagerMock) DeleteAll(ctx context.Context, namespace, accountName, accountID string) error { - args := m.Called(ctx, namespace, accountName, accountID) +func (m *secretManagerMock) DeleteAll(ctx context.Context, accountRef domain.NamespacedName, accountID string) error { + args := m.Called(ctx, accountRef, accountID) return args.Error(0) } -func (m *secretManagerMock) mockDeleteAll(ctx context.Context, namespace, accountName, accountID string) { - m.On("DeleteAll", ctx, namespace, accountName, accountID).Return(nil) +func (m *secretManagerMock) mockDeleteAll(ctx context.Context, accountRef domain.NamespacedName, accountID string) { + m.On("DeleteAll", ctx, accountRef, accountID).Return(nil) } -func (m *secretManagerMock) GetSecrets(ctx context.Context, namespace, accountName, accountID string) (*Secrets, error) { - args := m.Called(ctx, namespace, accountName, accountID) +func (m *secretManagerMock) GetSecrets(ctx context.Context, accountRef domain.NamespacedName, accountID string) (*Secrets, error) { + args := m.Called(ctx, accountRef, accountID) if args.Get(0) == nil { return nil, args.Error(1) } return args.Get(0).(*Secrets), args.Error(1) } -func (m *secretManagerMock) mockGetSecrets(ctx context.Context, namespace, accountName, accountID string, result *Secrets) { - m.On("GetSecrets", ctx, namespace, accountName, accountID).Return(result, nil) +func (m *secretManagerMock) mockGetSecrets(ctx context.Context, accountRef domain.NamespacedName, accountID string, result *Secrets) { + m.On("GetSecrets", ctx, accountRef, accountID).Return(result, nil) } -func (m *secretManagerMock) mockGetSecretsError(ctx context.Context, namespace, accountName, accountID string, err error) { - m.On("GetSecrets", ctx, namespace, accountName, accountID).Return(nil, err) +func (m *secretManagerMock) mockGetSecretsError(ctx context.Context, accountRef domain.NamespacedName, accountID string, err error) { + m.On("GetSecrets", ctx, accountRef, accountID).Return(nil, err) } var _ secretManager = (*secretManagerMock)(nil) diff --git a/internal/account/claims.go b/internal/account/claims.go index 0f421c3..27b37be 100644 --- a/internal/account/claims.go +++ b/internal/account/claims.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/jwt/v2" @@ -164,12 +165,12 @@ func newClaimsBuilder( imports := jwt.Imports{} for _, importClaim := range spec.Imports { - importAccount, err := accountReader.Get(ctx, importClaim.AccountRef.Name, importClaim.AccountRef.Namespace) + accountRef := domain.NewNamespacedName(importClaim.AccountRef.Namespace, importClaim.AccountRef.Name) + importAccount, err := accountReader.Get(ctx, accountRef) if err != nil { - errs = append(errs, fmt.Errorf("failed to get account for import %q (namespace: %q, account: %q): %w", + errs = append(errs, fmt.Errorf("failed to get account for import %q (account: %q): %w", importClaim.Name, - importClaim.AccountRef.Namespace, - importClaim.AccountRef.Name, + accountRef, err)) } else { account := importAccount.Labels[k8s.LabelAccountID] diff --git a/internal/account/claims_test.go b/internal/account/claims_test.go index 63b086a..19375a6 100644 --- a/internal/account/claims_test.go +++ b/internal/account/claims_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" approvals "github.com/approvals/go-approval-tests" "github.com/nats-io/jwt/v2" @@ -49,9 +50,9 @@ func TestClaims(t *testing.T) { ctx := context.Background() accountReaderMock := NewAccountReaderMock() - getAccountCall := accountReaderMock.On("Get", mock.Anything, mock.Anything, mock.Anything) + getAccountCall := accountReaderMock.On("Get", mock.Anything, mock.Anything) getAccountCall.RunFn = func(args mock.Arguments) { - accountID := fakeAccountId(args.String(1), args.String(2)) + accountID := fakeAccountId(args.Get(1).(domain.NamespacedName)) account := &v1alpha1.Account{} account.Labels = map[string]string{ k8s.LabelAccountID: accountID, @@ -158,8 +159,8 @@ func discoverTestCases(pattern string) []TestCaseInputFile { return testCases } -func fakeAccountId(accountNameRef string, namespace string) string { - return fmt.Sprintf("A%055s", strings.ToUpper(strings.ReplaceAll(accountNameRef+namespace, "-", ""))) +func fakeAccountId(accountRef domain.NamespacedName) string { + return fmt.Sprintf("A%055s", strings.ToUpper(strings.ReplaceAll(accountRef.Name+accountRef.Namespace, "-", ""))) } func loadAccountSpec(filePath string) (*v1alpha1.AccountSpec, error) { diff --git a/internal/account/cluster.go b/internal/account/cluster.go index ef089bb..06d79d4 100644 --- a/internal/account/cluster.go +++ b/internal/account/cluster.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/nkeys" @@ -64,10 +65,10 @@ func newClusterTargetResolverImpl( return nil, fmt.Errorf("config is required") } if config.OperatorNatsCluster != nil { - opClusterRef := ports.NamespacedName{ - Namespace: config.OperatorNatsCluster.ClusterRef.Namespace, - Name: config.OperatorNatsCluster.ClusterRef.Name, - } + opClusterRef := domain.NewNamespacedName( + config.OperatorNatsCluster.ClusterRef.Namespace, + config.OperatorNatsCluster.ClusterRef.Name, + ) if err := opClusterRef.Validate(); err != nil { return nil, fmt.Errorf("invalid operator cluster reference: %v", err) } @@ -105,7 +106,7 @@ func (r *clusterTargetResolverImpl) GetClusterTarget(ctx context.Context, accoun var result *clusterTarget var err error if accountClusterRef != nil { - acClusterRef := ports.NamespacedName{Namespace: accountClusterRef.Namespace, Name: accountClusterRef.Name} + acClusterRef := domain.NewNamespacedName(accountClusterRef.Namespace, accountClusterRef.Name) if err = acClusterRef.Validate(); err != nil { return nil, fmt.Errorf("invalid account cluster reference: %v", err) } @@ -128,7 +129,7 @@ func (r *clusterTargetResolverImpl) GetClusterTarget(ctx context.Context, accoun return result, nil } -func (r *clusterTargetResolverImpl) resolveTarget(ctx context.Context, clusterRef ports.NamespacedName) (*clusterTarget, error) { +func (r *clusterTargetResolverImpl) resolveTarget(ctx context.Context, clusterRef domain.NamespacedName) (*clusterTarget, error) { cluster, err := r.natsClusterReader.Get(ctx, clusterRef) if err != nil { return nil, fmt.Errorf("failed to resolve NATS cluster %s: %w", clusterRef, err) @@ -179,8 +180,9 @@ func (r *clusterTargetResolverImpl) resolveTargetFromImplicitLookup(ctx context. } func (r *clusterTargetResolverImpl) resolveSysAdminCreds(ctx context.Context, cluster *v1alpha1.NatsCluster) (*ports.NatsUserCreds, error) { - secretRef := cluster.Spec.SystemAccountUserCredsSecretRef - creds, err := r.resolveSecret(ctx, cluster.GetNamespace(), secretRef.Name, secretRef.Key) + secretKeyRef := cluster.Spec.SystemAccountUserCredsSecretRef + secretRef := domain.NewNamespacedName(cluster.GetNamespace(), secretKeyRef.Name) + creds, err := r.resolveSecret(ctx, secretRef, secretKeyRef.Key) if err != nil { return nil, fmt.Errorf("resolve system account user creds for secret %s/%s: %w", cluster.GetNamespace(), secretRef.Name, err) } @@ -192,7 +194,7 @@ func (r *clusterTargetResolverImpl) resolveSysAdminCreds(ctx context.Context, cl } // Deprecated: This method relies on legacy patterns and will sunset in a future release. -func (r *clusterTargetResolverImpl) resolveSysAdminCredsViaLabels(ctx context.Context, namespace string) (*ports.NatsUserCreds, error) { +func (r *clusterTargetResolverImpl) resolveSysAdminCredsViaLabels(ctx context.Context, namespace domain.Namespace) (*ports.NatsUserCreds, error) { labels := map[string]string{ k8s.LabelSecretType: k8s.SecretTypeSystemAccountUserCreds, } @@ -209,8 +211,9 @@ func (r *clusterTargetResolverImpl) resolveSysAdminCredsViaLabels(ctx context.Co } func (r *clusterTargetResolverImpl) resolveOperatorSigningKey(ctx context.Context, cluster *v1alpha1.NatsCluster) (ports.NatsOperatorSigningKey, error) { - secretRef := cluster.Spec.OperatorSigningKeySecretRef - keyData, err := r.resolveSecret(ctx, cluster.GetNamespace(), secretRef.Name, secretRef.Key) + secretKeyRef := cluster.Spec.OperatorSigningKeySecretRef + secretRef := domain.NewNamespacedName(cluster.GetNamespace(), secretKeyRef.Name) + keyData, err := r.resolveSecret(ctx, secretRef, secretKeyRef.Key) if err != nil { return nil, fmt.Errorf("resolve operator signing key for NatsCluster %s/%s: %w", cluster.GetNamespace(), cluster.GetName(), err) } @@ -222,7 +225,7 @@ func (r *clusterTargetResolverImpl) resolveOperatorSigningKey(ctx context.Contex } // Deprecated: This method relies on legacy patterns and will sunset in a future release. -func (r *clusterTargetResolverImpl) resolveOperatorSigningKeyViaLabels(ctx context.Context, namespace string) (ports.NatsOperatorSigningKey, error) { +func (r *clusterTargetResolverImpl) resolveOperatorSigningKeyViaLabels(ctx context.Context, namespace domain.Namespace) (ports.NatsOperatorSigningKey, error) { labels := map[string]string{k8s.LabelSecretType: k8s.SecretTypeOperatorSign} seed, err := r.resolveSecretByLabels(ctx, namespace, labels) if err != nil { @@ -235,7 +238,7 @@ func (r *clusterTargetResolverImpl) resolveOperatorSigningKeyViaLabels(ctx conte return keyPair, err } -func (r *clusterTargetResolverImpl) resolveSecretByLabels(ctx context.Context, namespace string, labels map[string]string) ([]byte, error) { +func (r *clusterTargetResolverImpl) resolveSecretByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) ([]byte, error) { secrets, err := r.secretReader.GetByLabels(ctx, namespace, labels) if err != nil { return nil, fmt.Errorf("get secrets by labels %v in namespace %q: %w", labels, namespace, err) @@ -253,10 +256,10 @@ func (r *clusterTargetResolverImpl) resolveSecretByLabels(ctx context.Context, n return value, nil } -func (r *clusterTargetResolverImpl) resolveSecret(ctx context.Context, namespace, name, key string) ([]byte, error) { - secretData, err := r.secretReader.Get(ctx, namespace, name) +func (r *clusterTargetResolverImpl) resolveSecret(ctx context.Context, namespacedName domain.NamespacedName, key string) ([]byte, error) { + secretData, err := r.secretReader.Get(ctx, namespacedName) if err != nil { - return nil, fmt.Errorf("resolve secret %s/%s: %w", namespace, name, err) + return nil, fmt.Errorf("resolve secret %s: %w", namespacedName, err) } if key == "" { @@ -265,7 +268,7 @@ func (r *clusterTargetResolverImpl) resolveSecret(ctx context.Context, namespace value, ok := secretData[key] if !ok { - return nil, fmt.Errorf("secret %s/%s does not contain key %q", namespace, name, key) + return nil, fmt.Errorf("secret %s does not contain key %q", namespacedName, key) } return []byte(value), nil @@ -278,30 +281,30 @@ func (r *clusterTargetResolverImpl) resolveNatsURL(ctx context.Context, cluster if cluster.Spec.URLFrom != nil { urlFromRef := cluster.Spec.URLFrom - namespace := urlFromRef.Namespace - if namespace == "" { - namespace = cluster.GetNamespace() + resourceRef := domain.NewNamespacedName(urlFromRef.Namespace, urlFromRef.Name) + if resourceRef.Namespace == "" { + resourceRef.Namespace = cluster.GetNamespace() } switch urlFromRef.Kind { case v1alpha1.URLFromKindConfigMap: - data, err := r.configMapReader.Get(ctx, namespace, urlFromRef.Name) + data, err := r.configMapReader.Get(ctx, resourceRef) if err != nil { - return "", fmt.Errorf("get ConfigMap %s/%s: %w", namespace, urlFromRef.Name, err) + return "", fmt.Errorf("get ConfigMap %s: %w", resourceRef, err) } if natsURL, ok := data[urlFromRef.Key]; ok { return natsURL, nil } - return "", fmt.Errorf("configMap %s/%s has no key %q", namespace, urlFromRef.Name, urlFromRef.Key) + return "", fmt.Errorf("configMap %s has no key %q", resourceRef, urlFromRef.Key) case v1alpha1.URLFromKindSecret: - data, err := r.secretReader.Get(ctx, namespace, urlFromRef.Name) + data, err := r.secretReader.Get(ctx, resourceRef) if err != nil { - return "", fmt.Errorf("get Secret %s/%s: %w", namespace, urlFromRef.Name, err) + return "", fmt.Errorf("get Secret %s: %w", resourceRef, err) } if natsURL, ok := data[urlFromRef.Key]; ok { return natsURL, nil } - return "", fmt.Errorf("secret %s/%s has no key %q", namespace, urlFromRef.Name, urlFromRef.Key) + return "", fmt.Errorf("secret %s has no key %q", resourceRef, urlFromRef.Key) default: return "", fmt.Errorf("unsupported urlFrom.kind %q", urlFromRef.Kind) } @@ -310,7 +313,7 @@ func (r *clusterTargetResolverImpl) resolveNatsURL(ctx context.Context, cluster return "", fmt.Errorf("neither url nor urlFrom is set") } -func (r *clusterTargetResolverImpl) validateAccountClusterRef(accountClusterRef ports.NamespacedName) error { +func (r *clusterTargetResolverImpl) validateAccountClusterRef(accountClusterRef domain.NamespacedName) error { opClusterRef := r.operatorClusterRef() if opClusterRef != nil && !r.config.OperatorNatsCluster.Optional && !opClusterRef.Equals(accountClusterRef) { return fmt.Errorf("account cluster reference %s does not match required operator cluster %s", accountClusterRef, opClusterRef) @@ -319,12 +322,10 @@ func (r *clusterTargetResolverImpl) validateAccountClusterRef(accountClusterRef return nil } -func (r *clusterTargetResolverImpl) operatorClusterRef() *ports.NamespacedName { +func (r *clusterTargetResolverImpl) operatorClusterRef() *domain.NamespacedName { if r.config == nil || r.config.OperatorNatsCluster == nil { return nil } - return &ports.NamespacedName{ - Namespace: r.config.OperatorNatsCluster.ClusterRef.Namespace, - Name: r.config.OperatorNatsCluster.ClusterRef.Name, - } + result := domain.NewNamespacedName(r.config.OperatorNatsCluster.ClusterRef.Namespace, r.config.OperatorNatsCluster.ClusterRef.Name) + return &result } diff --git a/internal/account/cluster_test.go b/internal/account/cluster_test.go index 001fa87..e8a0101 100644 --- a/internal/account/cluster_test.go +++ b/internal/account/cluster_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/jwt/v2" @@ -74,7 +75,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenOperatorClust opSignKey, sauCreds := t.generateSecrets() opSignSeed, _ := opSignKey.Seed() - t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, ports.NamespacedName{Namespace: "my-namespace", Name: "my-cluster"}, + t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, domain.NewNamespacedName("my-namespace", "my-cluster"), &v1alpha1.NatsCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "my-namespace", @@ -86,9 +87,9 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenOperatorClust SystemAccountUserCredsSecretRef: v1alpha1.SecretKeyReference{Name: "sau-creds"}, }, }) - t.secretClientMock.mockGet(t.ctx, "my-namespace", "op-sign-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "op-sign-secret"), map[string]string{k8s.DefaultSecretKeyName: string(opSignSeed)}) - t.secretClientMock.mockGet(t.ctx, "my-namespace", "sau-creds", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "sau-creds"), map[string]string{k8s.DefaultSecretKeyName: string(sauCreds.Creds)}) // When @@ -114,7 +115,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste } opSignKey, sauCreds := t.generateSecrets() opSignSeed, _ := opSignKey.Seed() - t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, ports.NamespacedName{Namespace: "ac-namespace", Name: "ac-cluster"}, + t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, domain.NewNamespacedName("ac-namespace", "ac-cluster"), &v1alpha1.NatsCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "ac-namespace", @@ -126,9 +127,9 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste SystemAccountUserCredsSecretRef: v1alpha1.SecretKeyReference{Name: "sau-creds"}, }, }) - t.secretClientMock.mockGet(t.ctx, "ac-namespace", "op-sign-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("ac-namespace", "op-sign-secret"), map[string]string{k8s.DefaultSecretKeyName: string(opSignSeed)}) - t.secretClientMock.mockGet(t.ctx, "ac-namespace", "sau-creds", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("ac-namespace", "sau-creds"), map[string]string{k8s.DefaultSecretKeyName: string(sauCreds.Creds)}) // When @@ -154,7 +155,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste opSignKey, sauCreds := t.generateSecrets() opSignSeed, _ := opSignKey.Seed() - t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, ports.NamespacedName{Namespace: "my-namespace", Name: "my-cluster"}, + t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, domain.NewNamespacedName("my-namespace", "my-cluster"), &v1alpha1.NatsCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "my-namespace", @@ -166,9 +167,9 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste SystemAccountUserCredsSecretRef: v1alpha1.SecretKeyReference{Name: "sau-creds"}, }, }) - t.secretClientMock.mockGet(t.ctx, "my-namespace", "op-sign-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "op-sign-secret"), map[string]string{k8s.DefaultSecretKeyName: string(opSignSeed)}) - t.secretClientMock.mockGet(t.ctx, "my-namespace", "sau-creds", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "sau-creds"), map[string]string{k8s.DefaultSecretKeyName: string(sauCreds.Creds)}) // When @@ -198,7 +199,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste } opSignKey, sauCreds := t.generateSecrets() opSignSeed, _ := opSignKey.Seed() - t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, ports.NamespacedName{Namespace: "ac-namespace", Name: "ac-cluster"}, + t.natsClusterResolverMock.mockGetNatsCluster(t.ctx, domain.NewNamespacedName("ac-namespace", "ac-cluster"), &v1alpha1.NatsCluster{ ObjectMeta: metav1.ObjectMeta{ Namespace: "ac-namespace", @@ -210,9 +211,9 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldSucceed_WhenAccountCluste SystemAccountUserCredsSecretRef: v1alpha1.SecretKeyReference{Name: "sau-creds"}, }, }) - t.secretClientMock.mockGet(t.ctx, "ac-namespace", "op-sign-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("ac-namespace", "op-sign-secret"), map[string]string{k8s.DefaultSecretKeyName: string(opSignSeed)}) - t.secretClientMock.mockGet(t.ctx, "ac-namespace", "sau-creds", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("ac-namespace", "sau-creds"), map[string]string{k8s.DefaultSecretKeyName: string(sauCreds.Creds)}) // When @@ -257,7 +258,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldFail_WhenOperatorClusterN } unitUnderTest := t.newUnitUnderTest(opClusterRef, false, "nats", "") - t.natsClusterResolverMock.mockGetNatsClusterError(t.ctx, ports.NamespacedName{Namespace: "my-namespace", Name: "my-cluster"}, + t.natsClusterResolverMock.mockGetNatsClusterError(t.ctx, domain.NewNamespacedName("my-namespace", "my-cluster"), fmt.Errorf("the root cause")) // When @@ -280,7 +281,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldFail_WhenAccountClusterNo Namespace: "ac-namespace", Name: "ac-cluster", } - t.natsClusterResolverMock.mockGetNatsClusterError(t.ctx, ports.NamespacedName{Namespace: "ac-namespace", Name: "ac-cluster"}, + t.natsClusterResolverMock.mockGetNatsClusterError(t.ctx, domain.NewNamespacedName("ac-namespace", "ac-cluster"), fmt.Errorf("the root cause")) // When @@ -303,7 +304,7 @@ func (t *ClusterTestSuite) Test_GetClusterTarget_ShouldFail_WhenAccountClusterRe result, err := unitUnderTest.GetClusterTarget(t.ctx, acClusterRef) // Then - require.ErrorContains(t.T(), err, "invalid account cluster reference: namespace is required") + require.ErrorContains(t.T(), err, "invalid account cluster reference: namespace required") require.Nil(t.T(), result) } @@ -335,7 +336,7 @@ func (t *ClusterTestSuite) Test_resolveNatsURL_ShouldSucceed_FromConfigMap() { // Given unitUnderTest := t.newUnitUnderTestWithDefaults().(*clusterTargetResolverImpl) - t.configMapResolverMock.mockGet(t.ctx, "my-namespace", "my-config", + t.configMapResolverMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "my-config"), map[string]string{"theNatsURL": "nats://custom-nats:4222"}) // When @@ -362,7 +363,7 @@ func (t *ClusterTestSuite) Test_resolveNatsURL_ShouldSucceed_FromConfigMapWithEx // Given unitUnderTest := t.newUnitUnderTestWithDefaults().(*clusterTargetResolverImpl) - t.configMapResolverMock.mockGet(t.ctx, "config-namespace", "my-config", + t.configMapResolverMock.mockGet(t.ctx, domain.NewNamespacedName("config-namespace", "my-config"), map[string]string{"theNatsURL": "nats://custom-nats:4222"}) // When @@ -390,7 +391,7 @@ func (t *ClusterTestSuite) Test_resolveNatsURL_ShouldSucceed_FromSecret() { // Given unitUnderTest := t.newUnitUnderTestWithDefaults().(*clusterTargetResolverImpl) - t.secretClientMock.mockGet(t.ctx, "my-namespace", "my-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("my-namespace", "my-secret"), map[string]string{"theNatsURL": "nats://custom-nats:4222"}) // When @@ -417,7 +418,7 @@ func (t *ClusterTestSuite) Test_resolveNatsURL_ShouldSucceed_FromSecretWithExpli // Given unitUnderTest := t.newUnitUnderTestWithDefaults().(*clusterTargetResolverImpl) - t.secretClientMock.mockGet(t.ctx, "config-namespace", "my-secret", + t.secretClientMock.mockGet(t.ctx, domain.NewNamespacedName("config-namespace", "my-secret"), map[string]string{"theNatsURL": "nats://custom-nats:4222"}) // When @@ -475,7 +476,7 @@ func (t *ClusterTestSuite) newUnitUnderTestWithDefaults() clusterTargetResolver return t.newUnitUnderTest(nil, false, "", "") } -func (t *ClusterTestSuite) newUnitUnderTest(opClusterRef *v1alpha1.NatsClusterRef, opClusterOptional bool, opNamespace string, defaultNatsURL string) clusterTargetResolver { +func (t *ClusterTestSuite) newUnitUnderTest(opClusterRef *v1alpha1.NatsClusterRef, opClusterOptional bool, opNamespace domain.Namespace, defaultNatsURL string) clusterTargetResolver { var operatorNatsCluster *OperatorNatsCluster var err error if opClusterRef != nil { diff --git a/internal/account/config.go b/internal/account/config.go index 1e4bbdd..022662c 100644 --- a/internal/account/config.go +++ b/internal/account/config.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" k8sval "k8s.io/apimachinery/pkg/api/validation" ) @@ -13,14 +14,14 @@ type Config struct { OperatorNatsCluster *OperatorNatsCluster // OperatorNamespace is the Kubernetes namespace where the operator is deployed. // TODO: [#102][#144] When sunsetting DefaultNatsURL, remove this field if it no longer serves a purpose. - OperatorNamespace string + OperatorNamespace domain.Namespace // DefaultNatsURL is a comma-separated list of NATS server URLs to use when OperatorNatsCluster is not configured. // Deprecated: This field is deprecated and will be removed in a future release. // TODO: [#102][#144] Sunset DefaultNatsURL (NATS_URL) DefaultNatsURL string } -func NewConfig(operatorNatsCluster *OperatorNatsCluster, operatorNamespace string, defaultNatsURL string) (*Config, error) { +func NewConfig(operatorNatsCluster *OperatorNatsCluster, operatorNamespace domain.Namespace, defaultNatsURL string) (*Config, error) { config := &Config{ OperatorNatsCluster: operatorNatsCluster, OperatorNamespace: operatorNamespace, @@ -42,8 +43,8 @@ func (c *Config) validate() error { } } if c.OperatorNamespace != "" { - if errs := k8sval.ValidateNamespaceName(c.OperatorNamespace, false); len(errs) > 0 { - return fmt.Errorf("invalid operator namespace %q: %s", c.OperatorNamespace, strings.Join(errs, ", ")) + if err := c.OperatorNamespace.Validate(); err != nil { + return fmt.Errorf("invalid operator namespace %q: %s", c.OperatorNamespace, err) } } if c.DefaultNatsURL != "" { diff --git a/internal/account/mocks_test.go b/internal/account/mocks_test.go index 193c149..6ce1fb9 100644 --- a/internal/account/mocks_test.go +++ b/internal/account/mocks_test.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/stretchr/testify/mock" @@ -33,25 +34,25 @@ func (s *SecretClientMock) mockApply(arguments ...interface{}) *mock.Call { return s.On("Apply", arguments...) } -func (s *SecretClientMock) Get(ctx context.Context, namespace string, name string) (map[string]string, error) { - args := s.Called(ctx, namespace, name) +func (s *SecretClientMock) Get(ctx context.Context, namespacedName domain.NamespacedName) (map[string]string, error) { + args := s.Called(ctx, namespacedName) return args.Get(0).(map[string]string), args.Error(1) } -func (s *SecretClientMock) mockGet(ctx context.Context, namespace string, name string, result map[string]string) { - s.On("Get", ctx, namespace, name).Return(result, nil) +func (s *SecretClientMock) mockGet(ctx context.Context, namespacedName domain.NamespacedName, result map[string]string) { + s.On("Get", ctx, namespacedName).Return(result, nil) } -func (s *SecretClientMock) mockGetError(namespace string, name string, err error) { - s.On("Get", mock.Anything, namespace, name).Return(map[string]string{}, err) +func (s *SecretClientMock) mockGetError(namespacedName domain.NamespacedName, err error) { + s.On("Get", mock.Anything, namespacedName).Return(map[string]string{}, err) } -func (s *SecretClientMock) GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*corev1.SecretList, error) { +func (s *SecretClientMock) GetByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) (*corev1.SecretList, error) { args := s.Called(ctx, namespace, labels) return args.Get(0).(*corev1.SecretList), args.Error(1) } -func (s *SecretClientMock) mockGetByLabels(namespace string, labels map[string]string, result *corev1.SecretList) { +func (s *SecretClientMock) mockGetByLabels(namespace domain.Namespace, labels map[string]string, result *corev1.SecretList) { s.On("GetByLabels", mock.Anything, namespace, labels).Return(result, nil) } @@ -62,7 +63,7 @@ type mockSecret struct { Value []byte } -func (s *SecretClientMock) mockGetByLabelsSimplified(namespace string, labels map[string]string, results []mockSecret) { +func (s *SecretClientMock) mockGetByLabelsSimplified(namespace domain.Namespace, labels map[string]string, results []mockSecret) { secretItems := make([]corev1.Secret, 0, len(results)) for i, r := range results { key := r.Key @@ -87,7 +88,7 @@ func (s *SecretClientMock) mockGetByLabelsSimplified(namespace string, labels ma secretItems = append(secretItems, corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: string(namespace), Labels: secretLabels, }, Data: map[string][]byte{ @@ -100,36 +101,36 @@ func (s *SecretClientMock) mockGetByLabelsSimplified(namespace string, labels ma s.mockGetByLabels(namespace, labels, secretList) } -func (s *SecretClientMock) mockGetByLabelsSimple(namespace string, labels map[string]string, key string, value []byte) { +func (s *SecretClientMock) mockGetByLabelsSimple(namespace domain.Namespace, labels map[string]string, key string, value []byte) { result := &corev1.SecretList{Items: []corev1.Secret{{Data: map[string][]byte{key: value}}}} s.mockGetByLabels(namespace, labels, result) } -func (s *SecretClientMock) Delete(ctx context.Context, namespace string, name string) error { - args := s.Called(ctx, namespace, name) +func (s *SecretClientMock) Delete(ctx context.Context, namespacedName domain.NamespacedName) error { + args := s.Called(ctx, namespacedName) return args.Error(0) } -func (s *SecretClientMock) DeleteByLabels(ctx context.Context, namespace string, labels map[string]string) error { +func (s *SecretClientMock) DeleteByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) error { args := s.Called(ctx, namespace, labels) return args.Error(0) } -func (s *SecretClientMock) mockDeleteByLabels(namespace string, labels map[string]string) { +func (s *SecretClientMock) mockDeleteByLabels(namespace domain.Namespace, labels map[string]string) { s.On("DeleteByLabels", mock.Anything, namespace, labels).Return(nil) } -func (s *SecretClientMock) Label(ctx context.Context, namespace, name string, labels map[string]string) error { - args := s.Called(ctx, namespace, name, labels) +func (s *SecretClientMock) Label(ctx context.Context, namespacedName domain.NamespacedName, labels map[string]string) error { + args := s.Called(ctx, namespacedName, labels) return args.Error(0) } -func (s *SecretClientMock) mockLabel(namespace, name string, labels map[string]string) { - s.On("Label", mock.Anything, namespace, name, labels).Return(nil) +func (s *SecretClientMock) mockLabel(namespacedName domain.NamespacedName, labels map[string]string) { + s.On("Label", mock.Anything, namespacedName, labels).Return(nil) } -func (s *SecretClientMock) mockLabelError(namespace, name string, labels map[string]string, err error) { - s.On("Label", mock.Anything, namespace, name, labels).Return(err) +func (s *SecretClientMock) mockLabelError(namespacedName domain.NamespacedName, labels map[string]string, err error) { + s.On("Label", mock.Anything, namespacedName, labels).Return(err) } var _ ports.SecretClient = (*SecretClientMock)(nil) @@ -234,8 +235,8 @@ func NewAccountReaderMock() *AccountReaderMock { return &AccountReaderMock{} } -func (a *AccountReaderMock) Get(ctx context.Context, accountRefName string, namespace string) (account *v1alpha1.Account, err error) { - args := a.Called(ctx, accountRefName, namespace) +func (a *AccountReaderMock) Get(ctx context.Context, accountRef domain.NamespacedName) (account *v1alpha1.Account, err error) { + args := a.Called(ctx, accountRef) anAccount := args.Get(0).(v1alpha1.Account) return &anAccount, args.Error(1) } @@ -253,7 +254,7 @@ func NewNatsClusterReaderMock() *NatsClusterReaderMock { return &NatsClusterReaderMock{} } -func (m *NatsClusterReaderMock) Get(ctx context.Context, clusterRef ports.NamespacedName) (*v1alpha1.NatsCluster, error) { +func (m *NatsClusterReaderMock) Get(ctx context.Context, clusterRef domain.NamespacedName) (*v1alpha1.NatsCluster, error) { args := m.Called(ctx, clusterRef) if args.Get(0) == nil { return nil, args.Error(1) @@ -261,11 +262,11 @@ func (m *NatsClusterReaderMock) Get(ctx context.Context, clusterRef ports.Namesp return args.Get(0).(*v1alpha1.NatsCluster), args.Error(1) } -func (m *NatsClusterReaderMock) mockGetNatsCluster(ctx context.Context, clusterRef ports.NamespacedName, result *v1alpha1.NatsCluster) { +func (m *NatsClusterReaderMock) mockGetNatsCluster(ctx context.Context, clusterRef domain.NamespacedName, result *v1alpha1.NatsCluster) { m.On("Get", ctx, clusterRef).Return(result, nil) } -func (m *NatsClusterReaderMock) mockGetNatsClusterError(ctx context.Context, clusterRef ports.NamespacedName, err error) { +func (m *NatsClusterReaderMock) mockGetNatsClusterError(ctx context.Context, clusterRef domain.NamespacedName, err error) { m.On("Get", ctx, clusterRef).Return(nil, err) } @@ -282,13 +283,13 @@ func NewConfigMapReaderMock() *ConfigMapReaderMock { return &ConfigMapReaderMock{} } -func (m *ConfigMapReaderMock) Get(ctx context.Context, namespace string, name string) (map[string]string, error) { - args := m.Called(ctx, namespace, name) +func (m *ConfigMapReaderMock) Get(ctx context.Context, namespacedName domain.NamespacedName) (map[string]string, error) { + args := m.Called(ctx, namespacedName) return args.Get(0).(map[string]string), args.Error(1) } -func (m *ConfigMapReaderMock) mockGet(ctx context.Context, namespace string, name string, result map[string]string) { - m.On("Get", ctx, namespace, name).Return(result, nil) +func (m *ConfigMapReaderMock) mockGet(ctx context.Context, namespacedName domain.NamespacedName, result map[string]string) { + m.On("Get", ctx, namespacedName).Return(result, nil) } var _ ports.ConfigMapReader = (*ConfigMapReaderMock)(nil) diff --git a/internal/account/secret.go b/internal/account/secret.go index a88f7ce..94d549a 100644 --- a/internal/account/secret.go +++ b/internal/account/secret.go @@ -9,6 +9,7 @@ import ( "io" "sync" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/nkeys" @@ -23,10 +24,10 @@ type Secrets struct { } type secretManager interface { - ApplyRootSecret(ctx context.Context, namespace, accountName string, rootKeyPair nkeys.KeyPair) error - ApplySignSecret(ctx context.Context, namespace, accountName, accountID string, signKeyPair nkeys.KeyPair) error - DeleteAll(ctx context.Context, namespace, accountName, accountID string) error - GetSecrets(ctx context.Context, namespace, accountName, accountID string) (*Secrets, error) + ApplyRootSecret(ctx context.Context, accountRef domain.NamespacedName, rootKeyPair nkeys.KeyPair) error + ApplySignSecret(ctx context.Context, accountRef domain.NamespacedName, accountID string, signKeyPair nkeys.KeyPair) error + DeleteAll(ctx context.Context, accountRef domain.NamespacedName, accountID string) error + GetSecrets(ctx context.Context, accountRef domain.NamespacedName, accountID string) (*Secrets, error) } type secretManagerImpl struct { @@ -43,36 +44,33 @@ func newSecretManagerImpl(secretClient ports.SecretClient) (*secretManagerImpl, }, nil } -func (m *secretManagerImpl) ApplyRootSecret(ctx context.Context, namespace, accountName string, rootKeyPair nkeys.KeyPair) error { +func (m *secretManagerImpl) ApplyRootSecret(ctx context.Context, accountRef domain.NamespacedName, rootKeyPair nkeys.KeyPair) error { accountID, err := rootKeyPair.PublicKey() if err != nil { return fmt.Errorf("failed to get public key from account root secret: %w", err) } - return m.applySecret(ctx, namespace, accountName, accountID, SecretNameAccountRootTemplate, k8s.SecretTypeAccountRoot, rootKeyPair) + return m.applySecret(ctx, accountRef, accountID, SecretNameAccountRootTemplate, k8s.SecretTypeAccountRoot, rootKeyPair) } -func (m *secretManagerImpl) ApplySignSecret(ctx context.Context, namespace, accountName, accountID string, signKeyPair nkeys.KeyPair) error { - return m.applySecret(ctx, namespace, accountName, accountID, SecretNameAccountSignTemplate, k8s.SecretTypeAccountSign, signKeyPair) +func (m *secretManagerImpl) ApplySignSecret(ctx context.Context, accountRef domain.NamespacedName, accountID string, signKeyPair nkeys.KeyPair) error { + return m.applySecret(ctx, accountRef, accountID, SecretNameAccountSignTemplate, k8s.SecretTypeAccountSign, signKeyPair) } -func (m *secretManagerImpl) applySecret(ctx context.Context, namespace, accountName, accountID, nameTemplate, secretType string, keyPair nkeys.KeyPair) error { - if namespace == "" { - return fmt.Errorf("account namespace cannot be empty") - } - if accountName == "" { - return fmt.Errorf("account name cannot be empty") +func (m *secretManagerImpl) applySecret(ctx context.Context, accountRef domain.NamespacedName, accountID, nameTemplate, secretType string, keyPair nkeys.KeyPair) error { + if err := accountRef.Validate(); err != nil { + return fmt.Errorf("invalid account reference %s: %w", accountRef, err) } if accountID == "" { return fmt.Errorf("account ID cannot be empty") } - secretName := fmt.Sprintf(nameTemplate, accountName, mustGenerateShortHashFromID(accountID)) + secretName := fmt.Sprintf(nameTemplate, accountRef.Name, mustGenerateShortHashFromID(accountID)) secretMeta := metav1.ObjectMeta{ Name: secretName, - Namespace: namespace, + Namespace: accountRef.Namespace, Labels: map[string]string{ k8s.LabelAccountID: accountID, - k8s.LabelAccountName: accountName, + k8s.LabelAccountName: accountRef.Name, k8s.LabelSecretType: secretType, k8s.LabelManaged: k8s.LabelManagedValue, }, @@ -89,17 +87,21 @@ func (m *secretManagerImpl) applySecret(ctx context.Context, namespace, accountN return nil } -func (m *secretManagerImpl) DeleteAll(ctx context.Context, namespace, accountName, accountID string) error { +func (m *secretManagerImpl) DeleteAll(ctx context.Context, accountRef domain.NamespacedName, accountID string) error { + if err := accountRef.Validate(); err != nil { + return fmt.Errorf("invalid account reference %s: %w", accountRef, err) + } + labels := map[string]string{ k8s.LabelAccountID: accountID, - k8s.LabelAccountName: accountName, + k8s.LabelAccountName: accountRef.Name, } // TODO: Consider looking up secrets and then deleting them explicitly // or delete first by only AccountID label and then by AccountName label, to ensure secrets are deleted even if // they are not labelled correctly. This also allows for better error handling and logging of which secrets were // attempted to be deleted. // TODO: Consider secrets labelled nauth.io/managed=true, should we only delete those? - return m.secretClient.DeleteByLabels(ctx, namespace, labels) + return m.secretClient.DeleteByLabels(ctx, accountRef.GetNamespace(), labels) } func mustGenerateShortHashFromID(ID string) string { @@ -116,27 +118,30 @@ func mustGenerateShortHashFromID(ID string) string { return hash } -func (m *secretManagerImpl) GetSecrets(ctx context.Context, namespace, accountName, accountID string) (*Secrets, error) { +func (m *secretManagerImpl) GetSecrets(ctx context.Context, accountRef domain.NamespacedName, accountID string) (*Secrets, error) { var err error + if err = accountRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid account reference %s: %w", accountRef, err) + } if accountID != "" { - secretsByAccountID, errByAccountID := m.getAccountSecretsByAccountID(ctx, namespace, accountID) + secretsByAccountID, errByAccountID := m.getAccountSecretsByAccountID(ctx, accountRef.GetNamespace(), accountID) if errByAccountID == nil { return m.validatedResult(secretsByAccountID, accountID) } err = fmt.Errorf("failed to get account secrets by account ID %q: %w", accountID, errByAccountID) } - secretsByAccountName, errByAccountName := m.getAccountSecretsByAccountName(ctx, namespace, accountName) + secretsByAccountName, errByAccountName := m.getAccountSecretsByAccountName(ctx, accountRef) if errByAccountName == nil { return m.validatedResult(secretsByAccountName, accountID) } - err = errors.Join(err, fmt.Errorf("failed to get account secrets by account name %q: %w", accountName, errByAccountName)) + err = errors.Join(err, fmt.Errorf("failed to get account secrets by account name %q: %w", accountRef.Name, errByAccountName)) - secretsBySecretName, errBySecretName := m.getDeprecatedAccountSecretsByName(ctx, namespace, accountName, accountID) + secretsBySecretName, errBySecretName := m.getDeprecatedAccountSecretsByName(ctx, accountRef, accountID) if errBySecretName == nil { return m.validatedResult(secretsBySecretName, accountID) } - err = errors.Join(err, fmt.Errorf("failed to get account secrets by secret name (deprecated) for account name %q: %w", accountName, errBySecretName)) + err = errors.Join(err, fmt.Errorf("failed to get account secrets by secret name (deprecated) for account name %q: %w", accountRef.Name, errBySecretName)) return nil, err } @@ -152,7 +157,7 @@ func (m *secretManagerImpl) validatedResult(result *Secrets, accountID string) ( return result, nil } -func (m *secretManagerImpl) getAccountSecretsByAccountID(ctx context.Context, namespace, accountID string) (*Secrets, error) { +func (m *secretManagerImpl) getAccountSecretsByAccountID(ctx context.Context, namespace domain.Namespace, accountID string) (*Secrets, error) { labels := map[string]string{ k8s.LabelAccountID: accountID, k8s.LabelManaged: k8s.LabelManagedValue, @@ -165,12 +170,12 @@ func (m *secretManagerImpl) getAccountSecretsByAccountID(ctx context.Context, na return m.getAccountSecretsFromK8sSecrets(k8sSecrets) } -func (m *secretManagerImpl) getAccountSecretsByAccountName(ctx context.Context, namespace, accountName string) (*Secrets, error) { +func (m *secretManagerImpl) getAccountSecretsByAccountName(ctx context.Context, accountRef domain.NamespacedName) (*Secrets, error) { labels := map[string]string{ - k8s.LabelAccountName: accountName, + k8s.LabelAccountName: accountRef.Name, k8s.LabelManaged: k8s.LabelManagedValue, } - k8sSecrets, err := m.secretClient.GetByLabels(ctx, namespace, labels) + k8sSecrets, err := m.secretClient.GetByLabels(ctx, accountRef.GetNamespace(), labels) if err != nil { return nil, err } @@ -200,7 +205,7 @@ func (m *secretManagerImpl) getAccountSecretsFromK8sSecrets(k8sSecrets *v1.Secre return m.toAccountSecrets(secrets) } -func (m *secretManagerImpl) getDeprecatedAccountSecretsByName(ctx context.Context, namespace, accountName, accountID string) (*Secrets, error) { +func (m *secretManagerImpl) getDeprecatedAccountSecretsByName(ctx context.Context, accountRef domain.NamespacedName, accountID string) (*Secrets, error) { logger := logf.FromContext(ctx) type goRoutineResult struct { @@ -210,31 +215,32 @@ func (m *secretManagerImpl) getDeprecatedAccountSecretsByName(ctx context.Contex var wg sync.WaitGroup ch := make(chan goRoutineResult, 2) + namespace := accountRef.GetNamespace() for _, s := range []struct { - secretName string + secretRef domain.NamespacedName secretType string }{ { - secretName: fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, accountName), + secretRef: namespace.WithName(fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, accountRef.Name)), secretType: k8s.SecretTypeAccountRoot, }, { - secretName: fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, accountName), + secretRef: namespace.WithName(fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, accountRef.Name)), secretType: k8s.SecretTypeAccountSign, }, } { wg.Add(1) - go func(secretName, secretType string) { + go func(secretRef domain.NamespacedName, secretType string) { result := goRoutineResult{} defer wg.Done() defer func() { if r := recover(); r != nil { - result.err = fmt.Errorf("recovered panicked go routine from trying to get secret %s/%s of type %s: %v", namespace, secretName, secretType, r) + result.err = fmt.Errorf("recovered panicked go routine from trying to get secret %s of type %s: %v", secretRef, secretType, r) ch <- result } }() - accountSecret, err := m.secretClient.Get(ctx, namespace, secretName) + accountSecret, err := m.secretClient.Get(ctx, secretRef) if err != nil { result.err = err ch <- result @@ -246,13 +252,13 @@ func (m *secretManagerImpl) getDeprecatedAccountSecretsByName(ctx context.Contex k8s.LabelSecretType: secretType, k8s.LabelManaged: k8s.LabelManagedValue, } - if err := m.secretClient.Label(ctx, namespace, secretName, labels); err != nil { - logger.Info("unable to label secret", "secretName", secretName, "namespace", namespace, "secretType", secretType, "error", err) + if err := m.secretClient.Label(ctx, secretRef, labels); err != nil { + logger.Info("unable to label secret", "secretRef", secretRef, "namespace", namespace, "secretType", secretType, "error", err) } accountSecret[k8s.LabelSecretType] = secretType result.secret = accountSecret ch <- result - }(s.secretName, s.secretType) + }(s.secretRef, s.secretType) } wg.Wait() @@ -274,7 +280,7 @@ func (m *secretManagerImpl) getDeprecatedAccountSecretsByName(ctx context.Contex } if len(secrets) < 2 { - return nil, fmt.Errorf("missing one or more deprecated secret(s) for account: %s/%s", namespace, accountName) + return nil, fmt.Errorf("missing one or more deprecated secret(s) for account: %s", accountRef) } return m.toAccountSecrets(secrets) diff --git a/internal/account/secret_test.go b/internal/account/secret_test.go index df56930..0281888 100644 --- a/internal/account/secret_test.go +++ b/internal/account/secret_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/nats-io/nkeys" "github.com/stretchr/testify/mock" @@ -47,11 +48,11 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldDoMultipleLookups() { k8s.LabelAccountName: "my-account", k8s.LabelManaged: k8s.LabelManagedValue, }, []mockSecret{}) - t.secretClientMock.mockGetError("account-namespace", "my-account-ac-root", fmt.Errorf("root_not_found")) - t.secretClientMock.mockGetError("account-namespace", "my-account-ac-sign", fmt.Errorf("sign_not_found")) + t.secretClientMock.mockGetError(domain.NewNamespacedName("account-namespace", "my-account-ac-root"), fmt.Errorf("root_not_found")) + t.secretClientMock.mockGetError(domain.NewNamespacedName("account-namespace", "my-account-ac-sign"), fmt.Errorf("sign_not_found")) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "my-account", "FAKE_ACCOUNT_ID") + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "my-account"), "FAKE_ACCOUNT_ID") // Then t.Error(err) @@ -82,7 +83,7 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldSucceed_LookupByAccountID }) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "account-name", rootPub) + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootPub) // Then t.NoError(err) @@ -115,7 +116,7 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldSucceed_LookupByAccountNa }) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "account-name", rootPub) + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootPub) // Then t.NoError(err) @@ -136,25 +137,27 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldSucceed_DeprecatedLookupB k8s.LabelAccountName: "my-account", k8s.LabelManaged: k8s.LabelManagedValue, }, []mockSecret{}) - t.secretClientMock.mockGet(t.ctx, "account-namespace", "my-account-ac-root", map[string]string{ + acRootRef := domain.NewNamespacedName("account-namespace", "my-account-ac-root") + t.secretClientMock.mockGet(t.ctx, acRootRef, map[string]string{ k8s.DefaultSecretKeyName: string(rootSeed), }) - t.secretClientMock.mockLabel("account-namespace", "my-account-ac-root", map[string]string{ + t.secretClientMock.mockLabel(acRootRef, map[string]string{ k8s.LabelAccountID: rootPub, k8s.LabelSecretType: k8s.SecretTypeAccountRoot, k8s.LabelManaged: k8s.LabelManagedValue, }) - t.secretClientMock.mockGet(t.ctx, "account-namespace", "my-account-ac-sign", map[string]string{ + acSignRef := domain.NewNamespacedName("account-namespace", "my-account-ac-sign") + t.secretClientMock.mockGet(t.ctx, acSignRef, map[string]string{ k8s.DefaultSecretKeyName: string(signSeed), }) - t.secretClientMock.mockLabel("account-namespace", "my-account-ac-sign", map[string]string{ + t.secretClientMock.mockLabel(acSignRef, map[string]string{ k8s.LabelAccountID: rootPub, k8s.LabelSecretType: k8s.SecretTypeAccountSign, k8s.LabelManaged: k8s.LabelManagedValue, }) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "my-account", rootPub) + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "my-account"), rootPub) // Then t.NoError(err) @@ -175,25 +178,27 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldSucceed_DeprecatedLookupB k8s.LabelAccountName: "my-account", k8s.LabelManaged: k8s.LabelManagedValue, }, []mockSecret{}) - t.secretClientMock.mockGet(t.ctx, "account-namespace", "my-account-ac-root", map[string]string{ + acRootRef := domain.NewNamespacedName("account-namespace", "my-account-ac-root") + t.secretClientMock.mockGet(t.ctx, acRootRef, map[string]string{ k8s.DefaultSecretKeyName: string(rootSeed), }) - t.secretClientMock.mockLabel("account-namespace", "my-account-ac-root", map[string]string{ + t.secretClientMock.mockLabel(acRootRef, map[string]string{ k8s.LabelAccountID: rootPub, k8s.LabelSecretType: k8s.SecretTypeAccountRoot, k8s.LabelManaged: k8s.LabelManagedValue, }) - t.secretClientMock.mockGet(t.ctx, "account-namespace", "my-account-ac-sign", map[string]string{ + acSignRef := domain.NewNamespacedName("account-namespace", "my-account-ac-sign") + t.secretClientMock.mockGet(t.ctx, acSignRef, map[string]string{ k8s.DefaultSecretKeyName: string(signSeed), }) - t.secretClientMock.mockLabelError("account-namespace", "my-account-ac-sign", map[string]string{ + t.secretClientMock.mockLabelError(acSignRef, map[string]string{ k8s.LabelAccountID: rootPub, k8s.LabelSecretType: k8s.SecretTypeAccountSign, k8s.LabelManaged: k8s.LabelManagedValue, }, fmt.Errorf("something went wrong")) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "my-account", rootPub) + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "my-account"), rootPub) // Then t.NoError(err) @@ -227,7 +232,7 @@ func (t *SecretManagerTestSuite) Test_GetSecrets_ShouldFail_WhenSecretRootPubKey }) // When - result, err := t.unitUnderTest.GetSecrets(t.ctx, "account-namespace", "account-name", rootPub) + result, err := t.unitUnderTest.GetSecrets(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootPub) // Then t.Error(err) @@ -252,7 +257,7 @@ func (t *SecretManagerTestSuite) Test_ApplyRootSecret_ShouldSucceed() { }).Return(nil) // When - err := t.unitUnderTest.ApplyRootSecret(t.ctx, "account-namespace", "account-name", rootKey) + err := t.unitUnderTest.ApplyRootSecret(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootKey) // Then t.NoError(err) @@ -283,7 +288,7 @@ func (t *SecretManagerTestSuite) Test_ApplySignSecret_ShouldSucceed() { }).Return(nil) // When - err := t.unitUnderTest.ApplySignSecret(t.ctx, "account-namespace", "account-name", rootPub, signKey) + err := t.unitUnderTest.ApplySignSecret(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootPub, signKey) // Then t.NoError(err) @@ -306,7 +311,7 @@ func (t *SecretManagerTestSuite) Test_DeleteAll_ShouldSucceed() { }) // When - err := t.unitUnderTest.DeleteAll(t.ctx, "account-namespace", "account-name", rootPub) + err := t.unitUnderTest.DeleteAll(t.ctx, domain.NewNamespacedName("account-namespace", "account-name"), rootPub) // Then t.NoError(err) diff --git a/internal/domain/k8s.go b/internal/domain/k8s.go new file mode 100644 index 0000000..eb435b3 --- /dev/null +++ b/internal/domain/k8s.go @@ -0,0 +1,66 @@ +package domain + +import ( + "fmt" + "strings" + + k8sval "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/types" +) + +type Namespace string + +func (n Namespace) WithName(name string) NamespacedName { + return NamespacedName{ + Namespace: string(n), + Name: name, + } +} + +func (n Namespace) Validate() error { + if strings.TrimSpace(string(n)) == "" { + return fmt.Errorf("value required") + } + if errs := k8sval.ValidateNamespaceName(string(n), false); len(errs) > 0 { + return fmt.Errorf("value invalid %q: %s", n, strings.Join(errs, ", ")) + } + return nil +} + +type NamespacedName types.NamespacedName + +func NewNamespacedName(namespace, name string) NamespacedName { + r := NamespacedName{ + Namespace: namespace, + Name: name, + } + return r +} + +func (n NamespacedName) GetNamespace() Namespace { + return Namespace(n.Namespace) +} + +func (n NamespacedName) Equals(other NamespacedName) bool { + return n.Namespace == other.Namespace && n.Name == other.Name +} + +func (n NamespacedName) Validate() error { + if n.Namespace == "" { + return fmt.Errorf("namespace required") + } + if err := Namespace(n.Namespace).Validate(); err != nil { + return fmt.Errorf("namespace invalid %q: %w", n.Namespace, err) + } + if n.Name == "" { + return fmt.Errorf("name required") + } + if errs := k8sval.NameIsDNSSubdomain(n.Name, false); len(errs) > 0 { + return fmt.Errorf("name invalid %q: %s", n.Name, strings.Join(errs, ", ")) + } + return nil +} + +func (n NamespacedName) String() string { + return types.NamespacedName(n).String() +} diff --git a/internal/k8s/account.go b/internal/k8s/account.go index 1367063..145fa86 100644 --- a/internal/k8s/account.go +++ b/internal/k8s/account.go @@ -3,8 +3,10 @@ package k8s import ( "context" "errors" + "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/ports" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -22,11 +24,14 @@ func NewAccountClient(client client.Client) *AccountClient { return ag } -// Gets the referenced Account +// Get the referenced Account // Requires the account to be reconciled and ready -func (a *AccountClient) Get(ctx context.Context, accountRefName string, namespace string) (account *v1alpha1.Account, err error) { +func (a *AccountClient) Get(ctx context.Context, accountRef domain.NamespacedName) (account *v1alpha1.Account, err error) { log := logf.FromContext(ctx) - key := client.ObjectKey{Namespace: namespace, Name: accountRefName} + if err = accountRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + key := client.ObjectKey{Namespace: accountRef.Namespace, Name: accountRef.Name} account = &v1alpha1.Account{} err = a.client.Get(ctx, key, account) @@ -35,7 +40,7 @@ func (a *AccountClient) Get(ctx context.Context, accountRefName string, namespac } if !isReady(account) { - log.Error(err, "account is not ready", "namespace", namespace, "accountName", accountRefName) + log.Error(err, "account is not ready", "accountRef", accountRef) return nil, errors.Join(ErrAccountNotReady, err) } diff --git a/internal/k8s/account_test.go b/internal/k8s/account_test.go index 9e9ec75..d1bfc5e 100644 --- a/internal/k8s/account_test.go +++ b/internal/k8s/account_test.go @@ -4,6 +4,7 @@ import ( "context" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -19,11 +20,13 @@ var _ = Describe("Account getter", func() { ) ctx := context.Background() + var accountRef domain.NamespacedName BeforeEach(func() { + accountRef = domain.NewNamespacedName(namespace, accountName) + Expect(accountRef.Validate()).To(Succeed()) By("creating account to fetch") - err := createAccount(namespace, accountName) - Expect(err).ToNot(HaveOccurred()) + Expect(createAccount(namespace, accountName)).To(Succeed()) }) AfterEach(func() { @@ -36,7 +39,7 @@ var _ = Describe("Account getter", func() { By("setting up a default account getter") accountGetter := NewAccountClient(k8sClient) By("getting the account") - fetchedAccount, err := accountGetter.Get(ctx, accountName, namespace) + fetchedAccount, err := accountGetter.Get(ctx, accountRef) By("verifying the fetched account") Expect(err).To(HaveOccurred()) @@ -52,7 +55,7 @@ var _ = Describe("Account getter", func() { Expect(err).ToNot(HaveOccurred()) By("getting the account") - fetchedAccount, err := accountGetter.Get(ctx, accountName, namespace) + fetchedAccount, err := accountGetter.Get(ctx, accountRef) By("verifying the fetched account") Expect(err).ToNot(HaveOccurred()) diff --git a/internal/k8s/cluster.go b/internal/k8s/cluster.go index 6e33dbb..dd7dc74 100644 --- a/internal/k8s/cluster.go +++ b/internal/k8s/cluster.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/ports" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -19,7 +20,7 @@ func NewNatsClusterClient(reader client.Reader) *NatsClusterClient { } } -func (c *NatsClusterClient) Get(ctx context.Context, clusterRef ports.NamespacedName) (*v1alpha1.NatsCluster, error) { +func (c *NatsClusterClient) Get(ctx context.Context, clusterRef domain.NamespacedName) (*v1alpha1.NatsCluster, error) { if err := clusterRef.Validate(); err != nil { return nil, fmt.Errorf("invalid NATS cluster reference %q: %w", clusterRef, err) } diff --git a/internal/k8s/cluster_test.go b/internal/k8s/cluster_test.go index 46e0f59..aff8dba 100644 --- a/internal/k8s/cluster_test.go +++ b/internal/k8s/cluster_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/WirelessCar/nauth/api/v1alpha1" - "github.com/WirelessCar/nauth/internal/ports" + "github.com/WirelessCar/nauth/internal/domain" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -41,7 +41,7 @@ func TestClusterClient_GetNatsCluster(t *testing.T) { Build() unitUnderTest := NewNatsClusterClient(reader) - result, err := unitUnderTest.Get(context.Background(), ports.NamespacedName{Namespace: "test-namespace", Name: "test-cluster"}) + result, err := unitUnderTest.Get(context.Background(), domain.NewNamespacedName("test-namespace", "test-cluster")) require.NoError(t, err) require.NotNil(t, result) @@ -59,7 +59,7 @@ func TestClusterClient_GetNatsCluster(t *testing.T) { Build() unitUnderTest := NewNatsClusterClient(reader) - result, err := unitUnderTest.Get(context.Background(), ports.NamespacedName{Namespace: "missing-namespace", Name: "missing-cluster"}) + result, err := unitUnderTest.Get(context.Background(), domain.NewNamespacedName("missing-namespace", "missing-cluster")) require.Error(t, err) require.Nil(t, result) diff --git a/internal/k8s/configmap/configmap.go b/internal/k8s/configmap/configmap.go index 84695fd..1e70959 100644 --- a/internal/k8s/configmap/configmap.go +++ b/internal/k8s/configmap/configmap.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" v1 "k8s.io/api/core/v1" @@ -39,14 +40,17 @@ func NewClient(c client.Client) *Client { // Get returns the ConfigMap data as a map of key to string value. // Keys from both Data and BinaryData are included. -func (c *Client) Get(ctx context.Context, namespace string, name string) (map[string]string, error) { +func (c *Client) Get(ctx context.Context, configMapRef domain.NamespacedName) (map[string]string, error) { + if err := configMapRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid ConfigMap reference %q: %w", configMapRef, err) + } cm := &v1.ConfigMap{} - key := client.ObjectKey{Namespace: namespace, Name: name} + key := client.ObjectKey{Namespace: configMapRef.Namespace, Name: configMapRef.Name} if err := c.client.Get(ctx, key, cm); err != nil { if apierrors.IsNotFound(err) { return nil, k8s.ErrNotFound } - return nil, fmt.Errorf("failed to get configmap: %w", err) + return nil, fmt.Errorf("failed to get ConfigMap %s: %w", configMapRef, err) } result := make(map[string]string) for k, v := range cm.Data { diff --git a/internal/k8s/configmap/configmap_test.go b/internal/k8s/configmap/configmap_test.go index 39d68b2..5b5e994 100644 --- a/internal/k8s/configmap/configmap_test.go +++ b/internal/k8s/configmap/configmap_test.go @@ -3,6 +3,7 @@ package configmap import ( "context" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -21,9 +22,12 @@ var _ = Describe("ConfigMap client", func() { ctx := context.Background() var cmClient *Client + var configMapRef domain.NamespacedName BeforeEach(func() { cmClient = NewClient(k8sClient) + configMapRef = domain.NewNamespacedName(namespace, configmapName) + Expect(configMapRef.Validate()).To(Succeed()) }) AfterEach(func() { @@ -32,8 +36,11 @@ var _ = Describe("ConfigMap client", func() { }) It("returns ErrNotFound when the ConfigMap does not exist", func() { + configMapRef := domain.NewNamespacedName(namespace, "non-existing-configmap") + Expect(configMapRef.Validate()).To(Succeed()) + By("getting a non-existing ConfigMap") - _, err := cmClient.Get(ctx, namespace, "non-existing-configmap") + _, err := cmClient.Get(ctx, configMapRef) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(k8s.ErrNotFound)) }) @@ -53,7 +60,7 @@ var _ = Describe("ConfigMap client", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) By("getting the ConfigMap") - data, err := cmClient.Get(ctx, namespace, configmapName) + data, err := cmClient.Get(ctx, configMapRef) Expect(err).ToNot(HaveOccurred()) Expect(data).To(HaveKeyWithValue("url", "nats://nats.example.com:4222")) Expect(data).To(HaveKeyWithValue("other", "value")) @@ -74,7 +81,7 @@ var _ = Describe("ConfigMap client", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) By("getting the ConfigMap") - data, err := cmClient.Get(ctx, namespace, configmapName) + data, err := cmClient.Get(ctx, configMapRef) Expect(err).ToNot(HaveOccurred()) Expect(data).To(HaveKeyWithValue("url", "nats://nats.example.com:4222")) Expect(data).To(HaveLen(1)) @@ -97,7 +104,7 @@ var _ = Describe("ConfigMap client", func() { Expect(k8sClient.Create(ctx, cm)).To(Succeed()) By("getting the ConfigMap") - data, err := cmClient.Get(ctx, namespace, configmapName) + data, err := cmClient.Get(ctx, configMapRef) Expect(err).ToNot(HaveOccurred()) Expect(data).To(HaveKeyWithValue("data-key", "data-value")) Expect(data).To(HaveKeyWithValue("binary-key", "binary-value")) diff --git a/internal/k8s/secret/secret.go b/internal/k8s/secret/secret.go index 42a89a7..fbcfc7c 100644 --- a/internal/k8s/secret/secret.go +++ b/internal/k8s/secret/secret.go @@ -5,6 +5,7 @@ import ( "fmt" "maps" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" v1 "k8s.io/api/core/v1" @@ -30,7 +31,8 @@ func (k *Client) Apply(ctx context.Context, owner metav1.Object, meta metav1.Obj if !isManagedSecret(&meta) { return fmt.Errorf("label %s not supplied by secret %s/%s", k8s.LabelManaged, meta.Namespace, meta.Name) } - currentSecret, err := k.getSecret(ctx, meta.GetNamespace(), meta.GetName()) + secretRef := domain.NewNamespacedName(meta.Namespace, meta.Name) + currentSecret, err := k.getSecret(ctx, secretRef) if err != nil { if !apierrors.IsNotFound(err) { return fmt.Errorf("failed to get secret: %w", err) @@ -103,8 +105,8 @@ func addOwnerReferenceIfNotExists(secret *v1.Secret, owner metav1.Object) error return nil } -func (k *Client) Get(ctx context.Context, namespace string, name string) (map[string]string, error) { - secret, err := k.getSecret(ctx, namespace, name) +func (k *Client) Get(ctx context.Context, secretRef domain.NamespacedName) (map[string]string, error) { + secret, err := k.getSecret(ctx, secretRef) if err != nil { if apierrors.IsNotFound(err) { return nil, k8s.ErrNotFound @@ -120,14 +122,14 @@ func (k *Client) Get(ctx context.Context, namespace string, name string) (map[st return secretData, nil } -func (k *Client) GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*v1.SecretList, error) { +func (k *Client) GetByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) (*v1.SecretList, error) { return k.getSecretsByLabels(ctx, namespace, labels) } -func (k *Client) Delete(ctx context.Context, namespace string, name string) error { +func (k *Client) Delete(ctx context.Context, secretRef domain.NamespacedName) error { log := logf.FromContext(ctx) - secret, err := k.getSecret(ctx, namespace, name) + secret, err := k.getSecret(ctx, secretRef) if err != nil { if apierrors.IsNotFound(err) { return nil @@ -135,7 +137,7 @@ func (k *Client) Delete(ctx context.Context, namespace string, name string) erro return fmt.Errorf("failed to get secret while deleting: %w", err) } - log.Info("Trying to delete secret", "secretName", name) + log.Info("Trying to delete secret", "secretRef", secretRef) if err := k.client.Delete(ctx, secret); err != nil { return fmt.Errorf("failed to delete secret: %w", err) } @@ -143,7 +145,7 @@ func (k *Client) Delete(ctx context.Context, namespace string, name string) erro return nil } -func (k *Client) DeleteByLabels(ctx context.Context, namespace string, labels map[string]string) error { +func (k *Client) DeleteByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) error { log := logf.FromContext(ctx) secrets, err := k.getSecretsByLabels(ctx, namespace, labels) @@ -164,8 +166,8 @@ func (k *Client) DeleteByLabels(ctx context.Context, namespace string, labels ma return nil } -func (k *Client) Label(ctx context.Context, namespace string, name string, labels map[string]string) error { - secret, err := k.getSecret(ctx, namespace, name) +func (k *Client) Label(ctx context.Context, secretRef domain.NamespacedName, labels map[string]string) error { + secret, err := k.getSecret(ctx, secretRef) if err != nil { return fmt.Errorf("failed to get secret: %w", err) } @@ -178,17 +180,21 @@ func (k *Client) Label(ctx context.Context, namespace string, name string, label return k.client.Update(ctx, secret) } -func (k *Client) getSecret(ctx context.Context, namespace string, name string) (*v1.Secret, error) { +func (k *Client) getSecret(ctx context.Context, secretRef domain.NamespacedName) (*v1.Secret, error) { + if err := secretRef.Validate(); err != nil { + return nil, fmt.Errorf("invalid namespaced name %q: %w", secretRef, err) + } + k8sSecret := &v1.Secret{} - key := client.ObjectKey{Namespace: namespace, Name: name} + key := client.ObjectKey{Namespace: secretRef.Namespace, Name: secretRef.Name} if err := k.client.Get(ctx, key, k8sSecret); err != nil { return nil, err } return k8sSecret, nil } -func (k *Client) getSecretsByLabels(ctx context.Context, namespace string, labels map[string]string) (*v1.SecretList, error) { +func (k *Client) getSecretsByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) (*v1.SecretList, error) { secretList := &v1.SecretList{} matchingLabelsListOption := client.MatchingLabels{} maps.Copy(matchingLabelsListOption, labels) diff --git a/internal/k8s/secret/secret_test.go b/internal/k8s/secret/secret_test.go index cd80666..9de1a72 100644 --- a/internal/k8s/secret/secret_test.go +++ b/internal/k8s/secret/secret_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -19,17 +20,20 @@ var _ = Describe("Secrets storer", func() { const namespace = "default" secretMeta := metav1.ObjectMeta{ Name: resourceName, - Namespace: namespace, + Namespace: string(namespace), Labels: map[string]string{ k8s.LabelManaged: k8s.LabelManagedValue, }, } ctx := context.Background() var secretStorer *Client + var secretRef domain.NamespacedName BeforeEach(func() { By("creating the custom resource for the Kind Account") secretStorer = NewClient(k8sClient) + secretRef = domain.NewNamespacedName(namespace, resourceName) + Expect(secretRef.Validate()).NotTo(HaveOccurred()) }) AfterEach(func() { @@ -45,7 +49,7 @@ var _ = Describe("Secrets storer", func() { Expect(err).ToNot(HaveOccurred()) By("Retrieving the secret") - fetchedSecret, err := secretStorer.Get(ctx, namespace, resourceName) + fetchedSecret, err := secretStorer.Get(ctx, secretRef) Expect(err).ToNot(HaveOccurred()) Expect(fetchedSecret).ToNot(BeNil()) Expect(fetchedSecret).To(Equal(secret)) @@ -56,7 +60,7 @@ var _ = Describe("Secrets storer", func() { Expect(err).ToNot(HaveOccurred()) By("Retrieving the updated secret") - newFetchedSecret, err := secretStorer.Get(ctx, namespace, resourceName) + newFetchedSecret, err := secretStorer.Get(ctx, secretRef) Expect(err).ToNot(HaveOccurred()) Expect(newFetchedSecret).ToNot(BeNil()) Expect(newFetchedSecret).To(Equal(newSecret)) @@ -79,10 +83,10 @@ var _ = Describe("Secrets storer", func() { newSecret := map[string]string{"key": "new value"} err = secretStorer.Apply(ctx, nil, secretMeta, newSecret) Expect(err).To(HaveOccurred()) - Expect(err).To(Equal(fmt.Errorf("existing secret %s/%s not managed by nauth", namespace, resourceName))) + Expect(err).To(Equal(fmt.Errorf("existing secret %s not managed by nauth", secretRef))) By("Retrieving the secret again to verify not mutated") - newFetchedSecret, err := secretStorer.Get(ctx, namespace, resourceName) + newFetchedSecret, err := secretStorer.Get(ctx, secretRef) Expect(err).ToNot(HaveOccurred()) Expect(newFetchedSecret).ToNot(BeNil()) Expect(newFetchedSecret).To(Equal(existingSecret)) @@ -97,13 +101,19 @@ var _ = Describe("Secrets storer", func() { map[string]string{k8s.LabelManaged: "false"})) It("should return success when deleting a non existing secret", func() { + nonExistingSecretRef := domain.NewNamespacedName(namespace, "non-existing-secret") + Expect(secretRef.Validate()).NotTo(HaveOccurred()) + By("Trying to delete a non-existing secret") - err := secretStorer.Delete(ctx, namespace, "non-existing-secret") + err := secretStorer.Delete(ctx, nonExistingSecretRef) Expect(err).ToNot(HaveOccurred()) }) It("should return an error when the secret does not exist", func() { + nonExistingSecretRef := domain.NewNamespacedName(namespace, "non-existing-secret") + Expect(secretRef.Validate()).NotTo(HaveOccurred()) + By("Trying to retrieve a non-existing secret") - _, err := secretStorer.Get(ctx, namespace, "non-existing-secret") + _, err := secretStorer.Get(ctx, nonExistingSecretRef) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(k8s.ErrNotFound)) }) @@ -114,11 +124,11 @@ var _ = Describe("Secrets storer", func() { Expect(err).ToNot(HaveOccurred()) By("Deleting the secret") - err = secretStorer.Delete(ctx, namespace, resourceName) + err = secretStorer.Delete(ctx, secretRef) Expect(err).ToNot(HaveOccurred()) By("Retrieving the deleted secret") - _, err = secretStorer.Get(ctx, namespace, resourceName) + _, err = secretStorer.Get(ctx, secretRef) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(k8s.ErrNotFound)) }) diff --git a/internal/ports/k8s.go b/internal/ports/k8s.go index f202661..a52ce99 100644 --- a/internal/ports/k8s.go +++ b/internal/ports/k8s.go @@ -2,55 +2,34 @@ package ports import ( "context" - "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" ) -type NamespacedName types.NamespacedName - -func (n NamespacedName) Equals(other NamespacedName) bool { - return n.Namespace == other.Namespace && n.Name == other.Name -} - -func (n NamespacedName) Validate() error { - if n.Name == "" { - return fmt.Errorf("name is required") - } - if n.Namespace == "" { - return fmt.Errorf("namespace is required") - } - return nil -} - -func (n NamespacedName) String() string { - return fmt.Sprintf("%s/%s", n.Namespace, n.Name) -} - type ConfigMapReader interface { - Get(ctx context.Context, namespace string, name string) (map[string]string, error) + Get(ctx context.Context, configMapRef domain.NamespacedName) (map[string]string, error) } type SecretReader interface { - Get(ctx context.Context, namespace string, name string) (map[string]string, error) - GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*v1.SecretList, error) + Get(ctx context.Context, secretRef domain.NamespacedName) (map[string]string, error) + GetByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) (*v1.SecretList, error) } type SecretClient interface { SecretReader Apply(ctx context.Context, owner metav1.Object, meta metav1.ObjectMeta, valueMap map[string]string) error - Delete(ctx context.Context, namespace string, name string) error - DeleteByLabels(ctx context.Context, namespace string, labels map[string]string) error - Label(ctx context.Context, namespace, name string, labels map[string]string) error + Delete(ctx context.Context, secretRef domain.NamespacedName) error + DeleteByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) error + Label(ctx context.Context, secretRef domain.NamespacedName, labels map[string]string) error } type AccountReader interface { - Get(ctx context.Context, accountRefName string, namespace string) (account *v1alpha1.Account, err error) + Get(ctx context.Context, accountRef domain.NamespacedName) (account *v1alpha1.Account, err error) } type NatsClusterReader interface { - Get(ctx context.Context, clusterRef NamespacedName) (*v1alpha1.NatsCluster, error) + Get(ctx context.Context, clusterRef domain.NamespacedName) (*v1alpha1.NatsCluster, error) } diff --git a/internal/user/mocks_test.go b/internal/user/mocks_test.go index f38b397..154bd0e 100644 --- a/internal/user/mocks_test.go +++ b/internal/user/mocks_test.go @@ -4,6 +4,7 @@ import ( "context" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/ports" "github.com/stretchr/testify/mock" corev1 "k8s.io/api/core/v1" @@ -11,10 +12,10 @@ import ( ) /* **************************************************** -* Secret storer +* ports.SecretClient Mock *****************************************************/ -func NewSecretStorerMock() *SecretClientMock { +func NewSecretClientMock() *SecretClientMock { return &SecretClientMock{} } @@ -22,42 +23,58 @@ type SecretClientMock struct { mock.Mock } -// ApplySecret implements ports.SecretStorer. +// Apply implements ports.SecretStorer. func (s *SecretClientMock) Apply(ctx context.Context, owner metav1.Object, meta metav1.ObjectMeta, valueMap map[string]string) error { args := s.Called(ctx, owner, meta, valueMap) return args.Error(0) } -// GetSecret implements ports.SecretStorer. -func (s *SecretClientMock) Get(ctx context.Context, namespace string, name string) (map[string]string, error) { - args := s.Called(ctx, namespace, name) +func (s *SecretClientMock) mockApply(ctx context.Context, owner interface{}, meta interface{}, valueMap interface{}) { + s.On("Apply", ctx, owner, meta, valueMap).Return(nil) +} + +// Get implements ports.SecretStorer. +func (s *SecretClientMock) Get(ctx context.Context, secretRef domain.NamespacedName) (map[string]string, error) { + args := s.Called(ctx, secretRef) return args.Get(0).(map[string]string), args.Error(1) } +func (s *SecretClientMock) mockGet(ctx context.Context, namespacedName domain.NamespacedName, result map[string]string) { + s.On("Get", ctx, namespacedName).Return(result, nil) +} + // GetByLabels implements ports.SecretStorer. -func (s *SecretClientMock) GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*corev1.SecretList, error) { +func (s *SecretClientMock) GetByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) (*corev1.SecretList, error) { args := s.Called(ctx, namespace, labels) return args.Get(0).(*corev1.SecretList), args.Error(1) } +func (s *SecretClientMock) mockGetByLabels(ctx context.Context, namespace domain.Namespace, labels interface{}, list *corev1.SecretList) { + s.On("GetByLabels", ctx, namespace, labels).Return(list, nil) +} + // DeleteSecret implements ports.SecretStorer. -func (s *SecretClientMock) Delete(ctx context.Context, namespace string, name string) error { - args := s.Called(ctx, namespace, name) +func (s *SecretClientMock) Delete(ctx context.Context, secretRef domain.NamespacedName) error { + args := s.Called(ctx, secretRef) return args.Error(0) } // DeleteSecret implements ports.SecretStorer. -func (s *SecretClientMock) DeleteByLabels(ctx context.Context, namespace string, labels map[string]string) error { +func (s *SecretClientMock) DeleteByLabels(ctx context.Context, namespace domain.Namespace, labels map[string]string) error { args := s.Called(ctx, namespace, labels) return args.Error(0) } // LabelSecret implements ports.SecretStorer. -func (s *SecretClientMock) Label(ctx context.Context, namespace, name string, labels map[string]string) error { - args := s.Called(ctx, namespace, name, labels) +func (s *SecretClientMock) Label(ctx context.Context, secretRef domain.NamespacedName, labels map[string]string) error { + args := s.Called(ctx, secretRef, labels) return args.Error(0) } +func (s *SecretClientMock) mockLabel(namespacedName domain.NamespacedName, labels map[string]string) { + s.On("Label", mock.Anything, namespacedName, labels).Return(nil) +} + // Compile-time assertion that implementation satisfies the ports interface var _ ports.SecretClient = &SecretClientMock{} @@ -73,11 +90,15 @@ func NewAccountReaderMock() *AccountReaderMock { return &AccountReaderMock{} } -func (a *AccountReaderMock) Get(ctx context.Context, accountRefName string, namespace string) (account *v1alpha1.Account, err error) { - args := a.Called(ctx, accountRefName, namespace) +func (a *AccountReaderMock) Get(ctx context.Context, accountRef domain.NamespacedName) (account *v1alpha1.Account, err error) { + args := a.Called(ctx, accountRef) anAccount := args.Get(0).(v1alpha1.Account) return &anAccount, args.Error(1) } +func (a *AccountReaderMock) mockGet(ctx context.Context, accountRef domain.NamespacedName, result v1alpha1.Account) *mock.Call { + return a.On("Get", ctx, accountRef).Return(result, nil) +} + // Compile-time assertion that implementation satisfies the ports interface var _ ports.AccountReader = &AccountReaderMock{} diff --git a/internal/user/user.go b/internal/user/user.go index f2541d3..799fa1f 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -7,6 +7,7 @@ import ( "sync" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/ports" "github.com/nats-io/jwt/v2" @@ -28,18 +29,23 @@ func NewManager(accountsReader ports.AccountReader, secretClient ports.SecretCli } func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) error { - account, err := u.accountsReader.Get(ctx, state.Spec.AccountName, state.Namespace) + userRef := domain.NewNamespacedName(state.Namespace, state.Name) + accountRef := domain.NewNamespacedName(state.Namespace, state.Spec.AccountName) + if err := accountRef.Validate(); err != nil { + return fmt.Errorf("invalid account reference %q: %w", accountRef, err) + } + account, err := u.accountsReader.Get(ctx, accountRef) if err != nil { return err } accountID := account.GetLabels()[k8s.LabelAccountID] if accountID == "" { - return fmt.Errorf("account %s/%s does not have an account ID yet", account.GetNamespace(), account.GetName()) + return fmt.Errorf("account %s does not have an account ID yet", accountRef) } - accountSigningKeyPair, err := u.getAccountSigningKeyPair(ctx, account.GetNamespace(), account.GetName(), accountID) + accountSigningKeyPair, err := u.getAccountSigningKeyPair(ctx, accountRef, accountID) if err != nil { - return fmt.Errorf("failed to get signing key secret %s/%s: %w", account.GetNamespace(), account.GetName(), err) + return fmt.Errorf("failed to get signing key secret %s: %w", accountRef, err) } accountSigningKeyPublicKey, err := accountSigningKeyPair.PublicKey() if err != nil { @@ -63,8 +69,7 @@ func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) erro build() userJwt, err := natsClaims.Encode(accountSigningKeyPair) if err != nil { - userResource := fmt.Sprintf("%s-%s", state.GetNamespace(), state.GetName()) - return fmt.Errorf("failed to sign user jwt for %s: %w", userResource, err) + return fmt.Errorf("failed to sign user jwt for %s: %w", userRef, err) } userCreds, err := jwt.FormatUserConfig(userJwt, userSeed) @@ -109,20 +114,24 @@ func (u *Manager) Delete(ctx context.Context, state *v1alpha1.User) error { log := logf.FromContext(ctx) log.Info("Delete user", "userName", state.GetName()) - err := u.secretClient.Delete(ctx, state.Namespace, state.GetUserSecretName()) + secretRef := domain.NewNamespacedName(state.Namespace, state.GetUserSecretName()) + if err := secretRef.Validate(); err != nil { + return fmt.Errorf("invalid secret reference %q: %w", secretRef, err) + } + err := u.secretClient.Delete(ctx, secretRef) if err != nil { - return fmt.Errorf("failed to delete user secret %s/%s: %w", state.Namespace, state.GetUserSecretName(), err) + return fmt.Errorf("failed to delete user secret %s: %w", secretRef, err) } return nil } -func (u *Manager) getAccountSigningKeyPair(ctx context.Context, namespace, accountName, accountID string) (nkeys.KeyPair, error) { - if keyPair, err := u.getAccountSigningKeyPairByAccountID(ctx, namespace, accountName, accountID); err == nil { +func (u *Manager) getAccountSigningKeyPair(ctx context.Context, accountRef domain.NamespacedName, accountID string) (nkeys.KeyPair, error) { + if keyPair, err := u.getAccountSigningKeyPairByAccountID(ctx, accountRef, accountID); err == nil { return keyPair, nil } - keyPair, err := u.getDeprecatedAccountSigningKeyPair(ctx, namespace, accountName, accountID) + keyPair, err := u.getDeprecatedAccountSigningKeyPair(ctx, accountRef, accountID) if err != nil { return nil, err } @@ -130,23 +139,23 @@ func (u *Manager) getAccountSigningKeyPair(ctx context.Context, namespace, accou return keyPair, nil } -func (u *Manager) getAccountSigningKeyPairByAccountID(ctx context.Context, namespace, accountName, accountID string) (nkeys.KeyPair, error) { +func (u *Manager) getAccountSigningKeyPairByAccountID(ctx context.Context, accountRef domain.NamespacedName, accountID string) (nkeys.KeyPair, error) { labels := map[string]string{ k8s.LabelAccountID: accountID, k8s.LabelSecretType: k8s.SecretTypeAccountSign, k8s.LabelManaged: k8s.LabelManagedValue, } - secrets, err := u.secretClient.GetByLabels(ctx, namespace, labels) + secrets, err := u.secretClient.GetByLabels(ctx, accountRef.GetNamespace(), labels) if err != nil { - return nil, fmt.Errorf("failed to get signing secret for account: %s-%s due to %w", namespace, accountName, err) + return nil, fmt.Errorf("failed to get signing secret for account: %s due to %w", accountRef, err) } if len(secrets.Items) < 1 { - return nil, fmt.Errorf("no signing secret found for account: %s-%s", namespace, accountName) + return nil, fmt.Errorf("no signing secret found for account: %s", accountRef) } if len(secrets.Items) > 1 { - return nil, fmt.Errorf("more than 1 signing secret found for account: %s-%s", namespace, accountName) + return nil, fmt.Errorf("more than 1 signing secret found for account: %s", accountRef) } seed, ok := secrets.Items[0].Data[k8s.DefaultSecretKeyName] @@ -164,7 +173,7 @@ func getDisplayName(user *v1alpha1.User) string { } // Todo: Almost identical to the one in account/account.go - refactor ? -func (u *Manager) getDeprecatedAccountSigningKeyPair(ctx context.Context, namespace, accountName, accountID string) (nkeys.KeyPair, error) { +func (u *Manager) getDeprecatedAccountSigningKeyPair(ctx context.Context, accountRef domain.NamespacedName, accountID string) (nkeys.KeyPair, error) { logger := logf.FromContext(ctx) type goRoutineResult struct { @@ -174,31 +183,32 @@ func (u *Manager) getDeprecatedAccountSigningKeyPair(ctx context.Context, namesp var wg sync.WaitGroup ch := make(chan goRoutineResult, 2) + namespace := accountRef.GetNamespace() for _, s := range []struct { - secretName string + secretRef domain.NamespacedName secretType string }{ { - secretName: fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, accountName), + secretRef: namespace.WithName(fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, accountRef.Name)), secretType: k8s.SecretTypeAccountRoot, }, { - secretName: fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, accountName), + secretRef: namespace.WithName(fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, accountRef.Name)), secretType: k8s.SecretTypeAccountSign, }, } { wg.Add(1) - go func(secretName, secretType string) { + go func(secretRef domain.NamespacedName, secretType string) { result := goRoutineResult{} defer wg.Done() defer func() { if r := recover(); r != nil { - result.err = fmt.Errorf("recovered panicked go routine from trying to get secret %s-%s of type %s: %v", namespace, secretName, secretType, r) + result.err = fmt.Errorf("recovered panicked go routine from trying to get secret %s of type %s: %v", secretRef, secretType, r) ch <- result } }() - accountSecret, err := u.secretClient.Get(ctx, namespace, secretName) + accountSecret, err := u.secretClient.Get(ctx, secretRef) if err != nil { result.err = err ch <- result @@ -210,13 +220,13 @@ func (u *Manager) getDeprecatedAccountSigningKeyPair(ctx context.Context, namesp k8s.LabelSecretType: secretType, k8s.LabelManaged: k8s.LabelManagedValue, } - if err := u.secretClient.Label(ctx, namespace, secretName, labels); err != nil { - logger.Info("unable to label secret", "secretName", secretName, "namespace", namespace, "secretType", secretType, "error", err) + if err := u.secretClient.Label(ctx, secretRef, labels); err != nil { + logger.Info("unable to label secret", "secretRef", secretRef, "secretType", secretType, "error", err) } accountSecret[k8s.LabelSecretType] = secretType result.secret = accountSecret ch <- result - }(s.secretName, s.secretType) + }(s.secretRef, s.secretType) } wg.Wait() @@ -239,12 +249,12 @@ func (u *Manager) getDeprecatedAccountSigningKeyPair(ctx context.Context, namesp accountSignSecret, ok := secrets[k8s.SecretTypeAccountSign] if !ok { - return nil, fmt.Errorf("no deprecated signing key found for account %s-%s", namespace, accountName) + return nil, fmt.Errorf("no deprecated signing key found for account %s", accountRef) } accountSignSecretSeed, ok := accountSignSecret[k8s.DefaultSecretKeyName] if !ok { - return nil, fmt.Errorf("no deprecated signing key seed found for account %s-%s", namespace, accountName) + return nil, fmt.Errorf("no deprecated signing key seed found for account %s", accountRef) } return nkeys.FromSeed([]byte(accountSignSecretSeed)) } diff --git a/internal/user/user_test.go b/internal/user/user_test.go index f265143..e6ef47e 100644 --- a/internal/user/user_test.go +++ b/internal/user/user_test.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/domain" "github.com/WirelessCar/nauth/internal/k8s" "github.com/nats-io/nkeys" . "github.com/onsi/ginkgo/v2" @@ -29,18 +30,18 @@ var _ = Describe("User manager", func() { ctx = context.Background() userManager *Manager accountReaderMock *AccountReaderMock - secretStorerMock *SecretClientMock + secretClientMock *SecretClientMock ) BeforeEach(func() { By("creating the user manager") - secretStorerMock = NewSecretStorerMock() + secretClientMock = NewSecretClientMock() accountReaderMock = NewAccountReaderMock() - userManager = NewManager(accountReaderMock, secretStorerMock) + userManager = NewManager(accountReaderMock, secretClientMock) }) AfterEach(func() { - secretStorerMock.AssertExpectations(GinkgoT()) + secretClientMock.AssertExpectations(GinkgoT()) accountReaderMock.AssertExpectations(GinkgoT()) }) @@ -49,7 +50,7 @@ var _ = Describe("User manager", func() { user := GetNewUser() By("providing a user specification without any specific configuration") - accountReaderMock.On("Get", ctx, accountName, accountNamespace).Return(*account, nil) + accountReaderMock.mockGet(ctx, domain.NewNamespacedName(accountNamespace, accountName), *account) By("mocking preexisting account keys & CR") accountSigningKeyPair, _ := nkeys.CreateAccount() @@ -63,12 +64,12 @@ var _ = Describe("User manager", func() { }, }, } - secretStorerMock.On("GetByLabels", ctx, accountNamespace, mock.Anything).Return(secretsList, nil) + secretClientMock.mockGetByLabels(ctx, accountNamespace, mock.Anything, secretsList) By("User credentials are stored") - secretStorerMock.On("Apply", ctx, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { + secretClientMock.mockApply(ctx, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { return s.GetName() == user.GetUserSecretName() && s.GetNamespace() == accountNamespace - }), mock.AnythingOfType("map[string]string")).Return(nil) + }), mock.AnythingOfType("map[string]string")) err := userManager.CreateOrUpdate(ctx, user) @@ -84,33 +85,33 @@ var _ = Describe("User manager", func() { account := GetExistingAccount() By("mocking the secret storer") - secretStorerMock.On("GetByLabels", mock.Anything, account.GetNamespace(), mock.Anything).Return(&corev1.SecretList{}, nil) + secretClientMock.mockGetByLabels(ctx, domain.Namespace(account.GetNamespace()), mock.Anything, &corev1.SecretList{}) accountKeyPair, _ := nkeys.CreateAccount() accountPublicKey, _ := accountKeyPair.PublicKey() accountSeed, _ := accountKeyPair.Seed() accountSecretValueMock := map[string]string{k8s.DefaultSecretKeyName: string(accountSeed)} accountSecretNameMock := fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, account.GetName()) - secretStorerMock.On("Get", mock.Anything, account.GetNamespace(), accountSecretNameMock).Return(accountSecretValueMock, nil) + secretClientMock.mockGet(ctx, domain.NewNamespacedName(account.GetNamespace(), accountSecretNameMock), accountSecretValueMock) accountSecretLabelsMock := map[string]string{ k8s.LabelAccountID: accountPublicKey, k8s.LabelSecretType: k8s.SecretTypeAccountRoot, k8s.LabelManaged: k8s.LabelManagedValue, } - secretStorerMock.On("Label", mock.Anything, account.GetNamespace(), accountSecretNameMock, accountSecretLabelsMock).Return(nil) + secretClientMock.mockLabel(domain.NewNamespacedName(account.GetNamespace(), accountSecretNameMock), accountSecretLabelsMock) accountSigningKeyPair, _ := nkeys.CreateAccount() accountSigningPublicKey, _ := accountSigningKeyPair.PublicKey() accountSigningSeed, _ := accountSigningKeyPair.Seed() accountSigningSecretValueMock := map[string]string{k8s.DefaultSecretKeyName: string(accountSigningSeed)} accountSigningSecretNameMock := fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, account.GetName()) - secretStorerMock.On("Get", mock.Anything, account.GetNamespace(), accountSigningSecretNameMock).Return(accountSigningSecretValueMock, nil) + secretClientMock.mockGet(ctx, domain.NewNamespacedName(account.GetNamespace(), accountSigningSecretNameMock), accountSigningSecretValueMock) accountSigningSecretLabelsMock := map[string]string{ k8s.LabelAccountID: accountPublicKey, k8s.LabelSecretType: k8s.SecretTypeAccountSign, k8s.LabelManaged: k8s.LabelManagedValue, } - secretStorerMock.On("Label", mock.Anything, account.GetNamespace(), accountSigningSecretNameMock, accountSigningSecretLabelsMock).Return(nil) + secretClientMock.mockLabel(domain.NewNamespacedName(account.GetNamespace(), accountSigningSecretNameMock), accountSigningSecretLabelsMock) By("mocking existing account") account.Status.SigningKey = v1alpha1.KeyInfo{ @@ -119,12 +120,13 @@ var _ = Describe("User manager", func() { account.Labels = map[string]string{ k8s.LabelAccountID: accountPublicKey, } - accountReaderMock.On("Get", ctx, accountName, accountNamespace).Return(*account, nil) + accountReaderMock.mockGet(ctx, domain.NewNamespacedName(accountNamespace, accountName), *account) By("mock storing user credentials") - secretStorerMock.On("Apply", mock.Anything, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { + + secretClientMock.mockApply(ctx, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { return s.GetName() == user.GetUserSecretName() && s.GetNamespace() == accountNamespace - }), mock.AnythingOfType("map[string]string")).Return(nil) + }), mock.AnythingOfType("map[string]string")) err := userManager.CreateOrUpdate(ctx, user) @@ -138,7 +140,7 @@ var _ = Describe("User manager", func() { user := GetNewUser() By("providing a user specification without any specific configuration") - accountReaderMock.On("Get", ctx, accountName, accountNamespace).Return(*account, nil).Twice() + accountReaderMock.mockGet(ctx, domain.NewNamespacedName(accountNamespace, accountName), *account).Twice() By("mocking preexisting account keys & CR") accountSigningKeyPair, _ := nkeys.CreateAccount() @@ -152,12 +154,12 @@ var _ = Describe("User manager", func() { }, }, } - secretStorerMock.On("GetByLabels", ctx, accountNamespace, mock.Anything).Return(secretsList, nil) + secretClientMock.mockGetByLabels(ctx, accountNamespace, mock.Anything, secretsList) By("User credentials are stored") - secretStorerMock.On("Apply", ctx, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { + secretClientMock.mockApply(ctx, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { return s.GetName() == user.GetUserSecretName() && s.GetNamespace() == accountNamespace - }), mock.AnythingOfType("map[string]string")).Return(nil) + }), mock.AnythingOfType("map[string]string")) err := userManager.CreateOrUpdate(ctx, user)