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

azure/auth: fix config path parsing #1871

Merged
merged 1 commit into from
May 23, 2023

Conversation

fairclothjm
Copy link
Contributor

Closes #1186

@fairclothjm fairclothjm added this to the 3.16.0 milestone May 22, 2023
Copy link
Contributor

@raymonstah raymonstah left a comment

Choose a reason for hiding this comment

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

should we add a test case for an invalid path too?

@fairclothjm
Copy link
Contributor Author

@raymonstah

should we add a test case for an invalid path too?

Not a bad idea, but I couldn't come up with a good way to test that. I am open to suggestions if anyone has any :-)

@fairclothjm fairclothjm merged commit 4c36cf3 into main May 23, 2023
7 checks passed
@fairclothjm fairclothjm deleted the VAULT-3698/azure-auth-config-path-fix branch May 23, 2023 13:43
@raymonstah
Copy link
Contributor

I was thinking of a unit test for the azureAuthBackendConfigBackendFromPath function:

func TestAzureAuthBackendConfigBackendFromPath(t *testing.T) {
	tcs := map[string]struct {
		path        string
		response    string
		expectedErr error
	}{
		"invalid-path": {
			path:        "foo/bar/baz",
			response:    "",
			expectedErr: fmt.Errorf("no backend found"),
		},
	}

	for name, tc := range tcs {
		t.Run(name, func(t *testing.T) {
			response, err := azureAuthBackendConfigBackendFromPath(tc.path)
			assert.Equal(t, tc.expectedErr, err)
			assert.Equal(t, tc.response, response)
		})
	}
}

@fairclothjm
Copy link
Contributor Author

@raymonstah Sure I can do that but in that case I think we should do some refactoring so that other engines can use this regex. Nothing stopping them now since almost everything is in the vault pkg but it would make more sense to move this to a common place and rename it. I will do that when I can find some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot Create Azure Auth Backend Config with custom path having "/"
3 participants