Refactor `EncryptionConfig` for AWSManagedControlPlane

[richardcase]

trafficstars

/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]

@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.

[slayer321]

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

[richardcase]

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]

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.

[richardcase]

@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]

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

[richardcase]

@slayer321 - how are you getting on with this?

[slayer321]

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]

@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]

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

[richardcase]

/lifecycle frozen

[richardcase]

/remove-lifecycle frozen

[k8s-triage-robot]

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