Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the fact that some GCP env vars are immune to WithDisallowEnvVars #250

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Fix the fact that some GCP env vars are immune to WithDisallowEnvVars #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions wrappers/gcpckms/gcpckms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (s *Wrapper) SetConfig(_ context.Context, opt ...wrapping.Option) (*wrappin
switch {
case os.Getenv(EnvGcpCkmsWrapperKeyRing) != "" && !opts.Options.WithDisallowEnvVars:
s.keyRing = os.Getenv(EnvGcpCkmsWrapperKeyRing)
case os.Getenv(EnvVaultGcpCkmsSealKeyRing) != "":
case os.Getenv(EnvVaultGcpCkmsSealKeyRing) != "" && !opts.Options.WithDisallowEnvVars:
s.keyRing = os.Getenv(EnvVaultGcpCkmsSealKeyRing)
case opts.withKeyRing != "":
s.keyRing = opts.withKeyRing
Expand All @@ -141,7 +141,7 @@ func (s *Wrapper) SetConfig(_ context.Context, opt ...wrapping.Option) (*wrappin
switch {
case os.Getenv(EnvGcpCkmsWrapperCryptoKey) != "" && !opts.Options.WithDisallowEnvVars:
s.cryptoKey = os.Getenv(EnvGcpCkmsWrapperCryptoKey)
case os.Getenv(EnvVaultGcpCkmsSealCryptoKey) != "":
case os.Getenv(EnvVaultGcpCkmsSealCryptoKey) != "" && !opts.Options.WithDisallowEnvVars:
s.cryptoKey = os.Getenv(EnvVaultGcpCkmsSealCryptoKey)
case opts.withCryptoKey != "":
s.cryptoKey = opts.withCryptoKey
Expand Down
46 changes: 45 additions & 1 deletion wrappers/gcpckms/gcpckms_acc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const (
// TestGcpKeyIdAfterConfig will test the result of calling the wrapper's KeyId()
// after it's configured with various options
func TestGcpKeyIdAfterConfig(t *testing.T) {
t.Parallel()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed as it didn't play nice with the new test case disabling env-vars

// Now test for cases where CKMS values are provided
checkAndSetEnvVars(t)
ctx := context.Background()
Expand Down Expand Up @@ -91,10 +90,55 @@ func TestGcpKeyIdAfterConfig(t *testing.T) {
}
})
}
}

// TestDisableEnv makes sure that we properly get all our settings from a configuration
// map instead of the environment variables
func TestDisableEnv(t *testing.T) {
// Now test for cases where CKMS values are provided
checkAndSetEnvVars(t)

configMap := map[string]string{
"project": os.Getenv(EnvGcpCkmsWrapperProject),
"region": os.Getenv(EnvGcpCkmsWrapperLocation),
"key_ring": os.Getenv(EnvGcpCkmsWrapperKeyRing),
"crypto_key": os.Getenv(EnvGcpCkmsWrapperCryptoKey),
}

// Reset the env values to validate we are using the config map ones
t.Setenv(EnvGcpCkmsWrapperProject, "bad_project")
t.Setenv(EnvGcpCkmsWrapperLocation, "bad_location")
t.Setenv(EnvGcpCkmsWrapperKeyRing, "bad_key_ring")
t.Setenv(EnvGcpCkmsWrapperCryptoKey, "bad_crypto_key")
t.Setenv(EnvVaultGcpCkmsSealKeyRing, "bad_vault_key_ring")
t.Setenv(EnvVaultGcpCkmsSealCryptoKey, "bad_vault_crypto_key")

s := NewWrapper()
_, err := s.SetConfig(context.Background(), wrapping.WithConfigMap(configMap), wrapping.WithDisallowEnvVars(true))
if err != nil {
t.Fatalf("got error from SetConfig %v", err)
}

// Make sure we can use the key properly.
input := []byte("foo")
swi, err := s.Encrypt(context.Background(), input)
if err != nil {
t.Fatalf("err: %s", err.Error())
}

pt, err := s.Decrypt(context.Background(), swi)
if err != nil {
t.Fatalf("err: %s", err.Error())
}

if !reflect.DeepEqual(input, pt) {
t.Fatalf("expected %s, got %s", input, pt)
}
}

func TestGcpCkmsSeal(t *testing.T) {
t.Setenv(EnvGcpCkmsWrapperProject, "") // Make sure at least one required value is not set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test worked correctly if it ran by itself but running them all together, if another test ran first and set the appropriate vars would fail as we wouldn't get the failure we expected.


// Do an error check before env vars are set
s := NewWrapper()
_, err := s.SetConfig(context.Background())
Expand Down
Loading