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

Refactor EncryptionConfig for AWSManagedControlPlane #2540

Open
richardcase opened this issue Jun 24, 2021 · 13 comments
Open

Refactor EncryptionConfig for AWSManagedControlPlane #2540

richardcase opened this issue Jun 24, 2021 · 13 comments
Labels
area/provider/eks Issues or PRs related to Amazon EKS provider help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/refactor lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@richardcase
Copy link
Member

/area provider/eks
/kind api-change
/kind refactor
/milestone v0.7.0
/help
/priority important-soon

Describe the solution you'd like
You can optionally enable encryption for EKS by supplying details in AWSManagedControlPlane.Spec.EncryptionConfig. As its optional, it is a pointer and marked as optional and omitempty.

If you want to enable encryption, then you must supply the provider and resources. Currently, these are pointers and not marked as required. We should mark these as required using kubebuilder validation and remove the pointers/omitempty.

This was noticed whilst making a change for #2505

Anything else you would like to add:
So perhaps something like this:

// EncryptionConfig specifies the encryption configuration for the EKS clsuter.
type EncryptionConfig struct {
	// Provider specifies the ARN or alias of the CMK (in AWS KMS)
	// +kubebuilder:validation:Required
	Provider string `json:"provider"`
	// Resources specifies the resources to be encrypted
	// +kubebuilder:validation:Required
	// +kubebuilder:validation:MinItems=1
	Resources []string `json:"resources"`
}

This will cause problems with the generated deepcopy and conversion functions which will need to be fixed.

Environment:

  • Cluster-api-provider-aws version: 0.6.6
@k8s-ci-robot
Copy link
Contributor

@richardcase:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/area provider/eks
/kind api-change
/kind refactor
/milestone v0.7.0
/help
/priority important-soon

Describe the solution you'd like
You can optionally enable encryption for EKS by supplying details in AWSManagedControlPlane.Spec.EncryptionConfig. As its optional, it is a pointer and marked as optional and omitempty.

If you want to enable encryption, then you must supply the provider and resources. Currently, these are pointers and not marked as required. We should mark these as required using kubebuilder validation and remove the pointers/omitempty.

This was noticed whilst making a change for #2505

Anything else you would like to add:
So perhaps something like this:

// EncryptionConfig specifies the encryption configuration for the EKS clsuter.
type EncryptionConfig struct {
  // Provider specifies the ARN or alias of the CMK (in AWS KMS)
  // +kubebuilder:validation:Required
  Provider string `json:"provider"`
  // Resources specifies the resources to be encrypted
  // +kubebuilder:validation:Required
  // +kubebuilder:validation:MinItems=1
  Resources []string `json:"resources"`
}

This will cause problems with the generated deepcopy and conversion functions which will need to be fixed.

Environment:

  • Cluster-api-provider-aws version: 0.6.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added this to the v0.7.0 milestone Jun 24, 2021
@k8s-ci-robot k8s-ci-robot added area/provider/eks Issues or PRs related to Amazon EKS provider kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/refactor priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 24, 2021
@slayer321
Copy link

Hii @richardcase, I am new to open source and will like to contribute to this project so can you assign it to me.

@richardcase
Copy link
Member Author

Welcome @slayer321, thanks for wanting to work on this issue. If you have any questions feel free to ask on here or in the slack channel.

/assign slayer321

@slayer321
Copy link

Hii @richardcase, I made changes in the EncryptionConfig struct as you have mentioned above but there are some errors that I am getting in generated deepcopy and conversion file and I am a bit confused about what exact changes need to be done in that file.

Ps: I am new to Golang so it is getting a bit difficult for me to understand the problem.

@randomvariable randomvariable modified the milestones: v0.7.0, v0.7.x Jun 28, 2021
@richardcase
Copy link
Member Author

@slayer321 - feel free to ping me on slack and we can chat.

Sometimes with the auto generated code you need to make manual changes so that the code is regenerated properly.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 29, 2021
@richardcase
Copy link
Member Author

@slayer321 - how are you getting on with this?

@slayer321
Copy link

slayer321 commented Sep 30, 2021

Hii @richardcase, Sorry due to some college work I was not able to work on this issue.
But now I'm looking to work on this issue.

As you mention ..... when I change the pointers in this struct there are some change in deepcopy and conversion function which I'm not able to understand can you point me to some resources where I can learn about it.

@richardcase
Copy link
Member Author

@slayer321 - when there are changes to the API definitions (like this change) then you need to run make generate. This does many things, some steps run various code generation tools that handle creating deepcopy functions or API conversions. If you run this and then perhaps ping me on slack?

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 30, 2021
@richardcase
Copy link
Member Author

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Oct 30, 2021
@randomvariable randomvariable modified the milestones: v0.7.x, Backlog Nov 8, 2021
@richardcase
Copy link
Member Author

/remove-lifecycle frozen

@k8s-ci-robot k8s-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Jul 12, 2022
@richardcase richardcase removed this from the Backlog milestone Jul 25, 2022
@slayer321 slayer321 removed their assignment Sep 28, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/provider/eks Issues or PRs related to Amazon EKS provider help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/refactor lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

No branches or pull requests

5 participants