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.

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

fix: handle different vault injection cases #75

Conversation

csatib02
Copy link
Member

@csatib02 csatib02 commented Feb 13, 2024

Overview

  • The injector already utilizes a bunch of things, to detect vault secret references.
  • On the side of secret-init it's sufficient if we use a regex to detect vault:something/something/... like substrings in the env-values, since it won't mistake vault like env-values for vault references, and if somehow a non-vault reference would be detected, the injector would just ignore it.
  • Added an e2e test case to test different injection scenarios.

Fixes #68

Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
@csatib02 csatib02 self-assigned this Feb 13, 2024
@github-actions github-actions bot added the size/S Denotes a PR that changes 10-99 lines label Feb 13, 2024
Signed-off-by: Bence Csati <bcsati@cisco.com>
@github-actions github-actions bot added size/M Denotes a PR that changes 100-499 lines and removed size/S Denotes a PR that changes 10-99 lines labels Feb 13, 2024
Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
Signed-off-by: Bence Csati <bcsati@cisco.com>
env_store.go Outdated Show resolved Hide resolved
env_store.go Outdated Show resolved Hide resolved
Signed-off-by: Bence Csati <bcsati@cisco.com>
@ramizpolic
Copy link
Member

The test scenario in env_store_test.go could be a bit changed to set envs itself, for example rather than using createEnvsForProvider helper function, we could set envs in the test cases explicitly.
Note that the actual file is not needed for this specific test case since the test is only extracting data from envs.
I have added three cases to cover, just make sure to fill with the expected data

func TestEnvStore_GetProviderPaths(t *testing.T) {
	tests := []struct {
		name      string
		envs 	  map[string]string
		wantPaths map[string][]string
	}{
		{
			name: "file provider",
			envs: map[string]string{
				"AWS_SECRET_ACCESS_KEY_ID": "file:FILEPATH",
			}
			wantPaths: map[string][]string{
				"file": {
					"FILEPATH"
				},
			},
		},
		{
			name: "vault provider",
			envs: map[string]string{
				// Add all the envs from https://github.com/bank-vaults/internal/blob/1d4670005e01baaa08bac7e17d0876d444d3286b/injector/injector_test.go#L92-L101
			}
			wantPaths: map[string][]string{
				"vault": {
					// fill expected
				},
			},
		},
		{
			name: "multi provider",
			envs: map[string]string{
				"AWS_SECRET_ACCESS_KEY_ID": "file:FILEPATH",
				"MYSQL_PASSWORD": "vault:secret/data/test/mysql#MYSQL_PASSWORD",
				"AWS_SECRET_ACCESS_KEY: "vault:secret/data/test/aws#AWS_SECRET_ACCESS_KEY",
			}
			wantPaths: map[string][]string{
				"vault": {
					// fill expected
				},
				"file": {
					// fill expected
				},
			},
		},
	}

	for _, tt := range tests {
		ttp := tt
		t.Run(ttp.name, func(t *testing.T) {
			// prepare envs
			for envKey, envVal := range ttp.envs {
				os.Setenv(envKey, envVal)
				t.Cleanup(func() {
					os.Unsetenv(envKey)
				})
			}

			paths := NewEnvStore().GetProviderPaths()

			for key, expectedSlice := range ttp.wantPaths {
				actualSlice, ok := paths[key]
				assert.True(t, ok, "Key not found in actual paths")
				assert.ElementsMatch(t, expectedSlice, actualSlice, "Slices for key %s do not match", key)
			}
		})
	}
}

Signed-off-by: Bence Csati <bcsati@cisco.com>
provider/vault/config_test.go Outdated Show resolved Hide resolved
provider/file/config_test.go Outdated Show resolved Hide resolved
env_store_test.go Outdated Show resolved Hide resolved
Signed-off-by: Bence Csati <bcsati@cisco.com>
Copy link
Member

@ramizpolic ramizpolic left a comment

Choose a reason for hiding this comment

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

great work! :shipit:

@csatib02 csatib02 merged commit faa8e41 into bank-vaults:main Feb 22, 2024
20 checks passed
@csatib02 csatib02 deleted the fix/handle-different-vault-provider-injection-cases branch February 22, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 100-499 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle different Vault provider injection cases
2 participants