-
Notifications
You must be signed in to change notification settings - Fork 25
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
Enable Golang-based Integration Tests #590
Conversation
rewrite integration tests in go instead of bash Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
fmt.Println("Algorithm:", algorithm) | ||
fmt.Println("---") | ||
println("Algorithm:", algorithm) | ||
println("---") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using the simpler println()
equivalent instead of fmt.Println()
— I see this as go inspiring from Rust, and I like it.
// ValidateEnvironment checks the application's runtime environment for essential | ||
// configuration settings, particularly focusing on the presence of the | ||
// 'VSECM_SYSTEM_NAMESPACE' environment variable. This variable is expected to | ||
// define the Kubernetes namespace in which the application is running, which is | ||
// crucial for the application's proper operation within a Kubernetes cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a lot of documentation to public methods.
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
continue | ||
} | ||
|
||
secretOnDisk, err := persistence.ReadFromDisk(key, rootKeyTriplet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t like passing root key triplet like this… I had to do it to fix a circular dependency; will create a follow-up PR to make the function signature cleaner.
@@ -35,36 +40,36 @@ var currentState = Status{ | |||
|
|||
// Increment is a method for the Status struct that increments the NumSecrets | |||
// field by 1 if the provided secret name is not found in the in-memory store. | |||
func (s *Status) Increment(name string) { | |||
func (s *Status) Increment(name string, loader func(name any) (any, bool)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, Passing loader
as an argument is not the best way to do this.
That’s also to fix circular dependency. — Will clean it up in a more targeted PR.
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
// Count how many times 'Running' appears in the command output. | ||
podCount := strings.Count(cmdOutput, "Running") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Man! This is “way” easier to do it in bash!.
And it is more readable and expressive too.
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
|
||
// WaitForExampleWorkload is a placeholder for the workload readiness check. | ||
// Placeholder for the workload readiness check. Replace this with your actual readiness check logic. | ||
func Wait(seconds time.Duration) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are wrappers around bash fns.
tryAll( | ||
eval.SecretEncryption, | ||
state.Cleanup, | ||
deploy.WorkloadUsingSDK, | ||
eval.SecretRegistration, | ||
eval.SecretDeletion, | ||
eval.SecretRegistrationAppend, | ||
eval.SecretRegistrationJSONFormat, | ||
eval.SecretRegistrationYAMLFormat, | ||
state.Cleanup, | ||
deploy.WorkloadUsingSidecar, | ||
eval.SecretRegistrationSidecar, | ||
eval.SecretDeletionSidecar, | ||
eval.SecretRegistrationAppendSidecar, | ||
eval.SecretRegistrationJSONFormatSidecar, | ||
eval.SecretRegistrationYAMLFormatSidecar, | ||
state.Cleanup, | ||
eval.InitContainer, | ||
state.Cleanup, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way, it’s easy to add/remove comment out individual tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡
Signed-off-by: Volkan Özçelik <ovolkan@vmware.com>
@v0lkan, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
# >/' SPDX-License-Identifier: BSD-2-Clause | ||
# */ | ||
|
||
# builder image | ||
FROM golang:1.20.1-alpine3.17 as builder | ||
FROM golang:1.22.0-alpine3.19 as builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the golang version.
Enable Golang-based Integration Tests
Description
This PR introduced Golang-based integration tests for more modularity and ease of management of VSecM integration test suites.
There is also considerable refactoring that I’ll annotate in the source code.
Behavior should not have changed.
Test Policy Compliance
Code Quality
to understand.
Documentation
Additional Comments
Include any additional comments or context about the PR here.
Checklist
Before you submit this PR, please make sure:
especially the test policy.
under the project’s license.
By submitting this pull request, you confirm that my contribution is made under
the terms of the project’s license and that you have the authority to grant
these rights.
Thank you for your contribution to VMware Secrets Manager
🐢⚡️!