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

Added unit tests for various key stores #363

Merged
merged 1 commit into from
May 15, 2023
Merged

Conversation

shtripat
Copy link
Contributor

@shtripat shtripat commented May 11, 2023

Purpose of this change is to enable testing of various backend key stores like Azure, GCP, HashiCorp Vault etc. The tests use command like flags e.g. aws.config, azure.config to get details to connect to the key store. This is done to make the changes generic and so code remains agnostic to addition of new keystores.

Also this way we are making sure all backend key store integrations work just fine and later when integrated with KES server and client making calls throughKES server, there wont be any surprises.

Copy link
Member

@aead aead left a comment

Choose a reason for hiding this comment

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

Currently your tests are order dependent. We have to be able to run each test separately. Like go test -v -fs.path=<path> -run="FS_Create"

Maybe combine all FS tests into:

func TestFS(t *testing.T) {
	config := edge.FSKeyStore{
		Path: "./keys",
	}
       // Init the store - like create keys that should be there etc.

        t.Run("create", ...)
        t.Run("delete", ...)
        // Add more tests
}

@shtripat shtripat changed the title Added unit tests for FS and Hashicorp Vault stores Added unit tests for various key stores May 12, 2023
@shtripat shtripat force-pushed the edge-tests branch 6 times, most recently from c75504b to b621ad6 Compare May 12, 2023 18:00
@shtripat shtripat marked this pull request as ready for review May 15, 2023 03:28
@shtripat shtripat requested a review from aead May 15, 2023 03:28
Copy link
Member

@aead aead left a comment

Choose a reason for hiding this comment

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

Apart from some minor outstandings LGTM.

@aead
Copy link
Member

aead commented May 15, 2023

Also, can you squash the commits into one and add a commit message explaining the purpose of this change and why things are done in a certain way - e.g. why we use the config file CLI flag approach etc. @shtripat

@shtripat
Copy link
Contributor Author

Also, can you squash the commits into one and add a commit message explaining the purpose of this change and why things are done in a certain way - e.g. why we use the config file CLI flag approach etc. @shtripat

Sure. Will do that.

Purpose of this change is to enable testing of various backend key stores
like Azure, GCP, HashiCorp Vault etc. The tests use command like flags e.g.
`aws.config`, `azure.config` to get details to connect to the key store. This
is done to make the changes generic and so code remains agnostic to addition
of new keystores.
Also this way we are making sure all backend key store integrations work just
fine and later when integrated with KES server and client making calls through
KES server, there wont be ny surprises.

Signed-off-by: Shubhendu Ram Tripathi <[email protected]>
@aead aead merged commit 665f4a0 into minio:master May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants