From 77e2fd60159074b382ba5be67f2ff431120f1e4d Mon Sep 17 00:00:00 2001 From: George Leontiev Date: Mon, 5 Feb 2024 17:14:58 +0000 Subject: [PATCH] Address PR feedback --- secrets/errors.go | 2 +- secrets/secrets.go | 24 ++++---- secrets/secrets_test.go | 123 ++++++++++++++++++++++++++++++---------- 3 files changed, 105 insertions(+), 44 deletions(-) diff --git a/secrets/errors.go b/secrets/errors.go index fdc805dbe..89c711739 100644 --- a/secrets/errors.go +++ b/secrets/errors.go @@ -46,7 +46,7 @@ type SecretWrongTypeError struct { func (e SecretWrongTypeError) Error() string { return fmt.Sprintf( - "secrets: requested secret at path '%s' of type '%s' does not exist, but a secret of type '%s' does. Consider using the correct API to retrieve the secret", + "secrets: requested secret at path %q of type '%s' does not exist, but a secret of type '%s' does, consider using the correct API to retrieve the secret", e.Path, e.DeclaredType, e.CorrectType, diff --git a/secrets/secrets.go b/secrets/secrets.go index 9aeaadc02..199736e48 100644 --- a/secrets/secrets.go +++ b/secrets/secrets.go @@ -49,16 +49,16 @@ func (s *Secrets) GetSimpleSecret(path string) (SimpleSecret, error) { } secret, ok := s.simpleSecrets[path] if !ok { - _, vOk := s.versionedSecrets[path] - if vOk { + _, ok := s.versionedSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: SimpleType, CorrectType: VersionedType, } } - _, cOk := s.credentialSecrets[path] - if cOk { + _, ok = s.credentialSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: SimpleType, @@ -78,16 +78,16 @@ func (s *Secrets) GetVersionedSecret(path string) (VersionedSecret, error) { } secret, ok := s.versionedSecrets[path] if !ok { - _, sOk := s.simpleSecrets[path] - if sOk { + _, ok := s.simpleSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: VersionedType, CorrectType: SimpleType, } } - _, cOk := s.credentialSecrets[path] - if cOk { + _, ok = s.credentialSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: VersionedType, @@ -108,16 +108,16 @@ func (s *Secrets) GetCredentialSecret(path string) (CredentialSecret, error) { } secret, ok := s.credentialSecrets[path] if !ok { - _, sOk := s.simpleSecrets[path] - if sOk { + _, ok := s.simpleSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: CredentialType, CorrectType: SimpleType, } } - _, vOk := s.versionedSecrets[path] - if vOk { + _, ok = s.versionedSecrets[path] + if ok { return secret, SecretWrongTypeError{ Path: path, DeclaredType: CredentialType, diff --git a/secrets/secrets_test.go b/secrets/secrets_test.go index 1d48af983..5dae7b8b3 100644 --- a/secrets/secrets_test.go +++ b/secrets/secrets_test.go @@ -145,38 +145,99 @@ func TestSecretsWrongType(t *testing.T) { } } ` - buf := bytes.NewBuffer([]byte(rawSecrets)) - secrets, err := NewSecrets(buf) - if err != nil { - t.Fatal(err) - } - - _, err = secrets.GetSimpleSecret("secret/myservice/external-account-key") - - if err == nil || err.Error() != "secrets: requested secret at path 'secret/myservice/external-account-key' of type 'simple' does not exist, but a secret of type 'versioned' does. Consider using the correct API to retrieve the secret" { - t.Fatalf("expected error %v, actual: %v", SecretWrongTypeError{ - Path: "secret/myservice/external-account-key", - DeclaredType: "simple", - CorrectType: "versioned", - }, err) - } - - _, err = secrets.GetVersionedSecret("secret/myservice/some-api-key") - if err == nil || err.Error() != "secrets: requested secret at path 'secret/myservice/some-api-key' of type 'versioned' does not exist, but a secret of type 'simple' does. Consider using the correct API to retrieve the secret" { - t.Fatalf("expected error %v, actual: %v", SecretWrongTypeError{ - Path: "secret/myservice/some-api-key", - DeclaredType: "versioned", - CorrectType: "simple", - }, err) + tests := []struct { + name string + input string + function func(*Secrets) (interface{}, error) + expectedError error + }{ + { + name: "Simple vs Versioned", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetSimpleSecret("secret/myservice/external-account-key") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/external-account-key", + DeclaredType: "simple", + CorrectType: "versioned", + }, + }, + { + name: "Versioned vs Simple", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetVersionedSecret("secret/myservice/some-api-key") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/some-api-key", + DeclaredType: "versioned", + CorrectType: "simple", + }, + }, + { + name: "Credential vs Simple", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetCredentialSecret("secret/myservice/some-api-key") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/some-api-key", + DeclaredType: "credential", + CorrectType: "simple", + }, + }, + { + name: "Simple vs Credential", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetSimpleSecret("secret/myservice/some-database-credentials") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/some-database-credentials", + DeclaredType: "simple", + CorrectType: "credential", + }, + }, + { + name: "Versioned vs Credential", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetVersionedSecret("secret/myservice/some-database-credentials") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/some-database-credentials", + DeclaredType: "versioned", + CorrectType: "credential", + }, + }, + { + name: "Credential vs Versioned", + input: rawSecrets, + function: func(s *Secrets) (interface{}, error) { + return s.GetCredentialSecret("secret/myservice/external-account-key") + }, + expectedError: SecretWrongTypeError{ + Path: "secret/myservice/external-account-key", + DeclaredType: "credential", + CorrectType: "versioned", + }, + }, } - - _, err = secrets.GetCredentialSecret("secret/myservice/some-api-key") - if err == nil || err.Error() != "secrets: requested secret at path 'secret/myservice/some-api-key' of type 'credential' does not exist, but a secret of type 'simple' does. Consider using the correct API to retrieve the secret" { - t.Fatalf("expected error %v, actual: %v", SecretWrongTypeError{ - Path: "secret/myservice/some-api-key", - DeclaredType: "credential", - CorrectType: "simple", - }, err) + for _, tt := range tests { + tt := tt // capture range variable for parallel testing + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + buf := bytes.NewBuffer([]byte(tt.input)) + secrets, err := NewSecrets(buf) + if tt.expectedError == nil && err != nil { + t.Fatal(err) + } + _, err = tt.function(secrets) + if tt.expectedError != nil && err.Error() != tt.expectedError.Error() { + t.Fatalf("expected error %v, actual: %v", tt.expectedError, err) + } + }) } }