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

Mockgcp test for autokey config #2755

Closed

Conversation

nb-goog
Copy link
Contributor

@nb-goog nb-goog commented Sep 20, 2024

Add mockgcp tests for kms autokeyconfig resource

  • Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

@yuwenma
Copy link
Collaborator

yuwenma commented Sep 23, 2024

@nb-goog Can you follow this guide https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/docs/archive/README.NewResourceFromTerraform.md#add-necessary-yaml-files-to-testdata to add the basic and full test suites? This is the requirements to kick off a PR review and validate the Direct resource development as described here https://github.com/GoogleCloudPlatform/k8s-config-connector/tree/master/docs/develop-resources/scenarios

apis/kms/v1alpha1/kmsautokeyconfig_types.go Outdated Show resolved Hide resolved
apis/kms/v1alpha1/kmsautokeyconfig_types.go Outdated Show resolved Hide resolved
apis/kms/v1alpha1/kmsautokeyconfig_types.go Outdated Show resolved Hide resolved
apis/kms/v1alpha1/kmsautokeyconfig_types.go Outdated Show resolved Hide resolved
mockgcp/mockkms/autokeyconfig.go Show resolved Hide resolved
mockgcp/mockkms/service.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/test/resourcefixture/contexts/kms_context.go Outdated Show resolved Hide resolved
pkg/test/resourcefixture/contexts/kms_context.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

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

Looks great! A few nits and I think we want to add a test case to capture the keyProject changing to "UNINITIALIZED".

pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/autokeyconfig_controller.go Outdated Show resolved Hide resolved
pkg/controller/direct/kms/mapper.generated.go Outdated Show resolved Hide resolved
@nb-goog
Copy link
Contributor Author

nb-goog commented Oct 17, 2024

That will be scenario based testing right?
if so, could you share the resources...

Copy link
Collaborator

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot removed the lgtm label Oct 23, 2024
@yuwenma
Copy link
Collaborator

yuwenma commented Oct 23, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 23, 2024
@google-oss-prow google-oss-prow bot removed the lgtm label Oct 23, 2024
@yuwenma
Copy link
Collaborator

yuwenma commented Oct 23, 2024

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Oct 23, 2024
@google-oss-prow google-oss-prow bot removed the lgtm label Oct 24, 2024
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@nb-goog nb-goog mentioned this pull request Oct 24, 2024
2 tasks
@nb-goog nb-goog closed this Oct 25, 2024
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.

7 participants