✨Add missing Fields to RosaControlPlane - tags, etcdEncryption #4802
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/M
|
/cc @stevekuznetsov |
|
Hi @xiangjingli. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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
added
the
needs-ok-to-test
|
@muraee @stevekuznetsov @serngawy FYI Please review the new PR I created, where I addressed all your comments |
xiangjingli
force-pushed
the
newROSAControlPlaneSpec2
branch
3 times, most recently
from
2c65bf7 to
9fb9946
Compare
| tagsList := map[string]string{} | ||
|
|
||
| if len(rosaScope.ControlPlane.Spec.AdditionalTags) > 0 { | ||
| for tagKey, tagVal := range rosaScope.ControlPlane.Spec.AdditionalTags { | ||
| tagsList[strings.TrimSpace(tagKey)] = strings.TrimSpace(tagVal) | ||
| } | ||
| } | ||
|
|
||
| rosaScope.ControlPlane.Spec.AdditionalTags = tagsList |
There was a problem hiding this comment.
this is not needed with the validation in the webhook.
Signed-off-by: Xiangjing Li <[email protected]>
xiangjingli
force-pushed
the
newROSAControlPlaneSpec2
branch
from
9fb9946 to
c3e886c
Compare
|
|
||
| // The etcd encryption kms key ARN is the key used to encrypt etcd. | ||
| // It is only allowed for hosted cp and it needs to be pre-created in AWS KMS with tag red-hat:true. | ||
| // +optional |
There was a problem hiding this comment.
If we ditch the webhook, this is simply:
// +kubebuilder:validation:MaxLength=2048
// +kubebuilder:validation:XValidation:rule=`self.matches('^arn:aws[\\w-]*:kms:[\\w-]+:\\d{12}:key\\/(mrk-[0-9a-f]{32}$|[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$)')`, message="etcdEncryptionKMSArn must be a valid encryption key ARN"
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/ok-to-test |
|
Superseded by #4832 |
|
@stevekuznetsov: Closed this PR. In response to this:
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. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adding the missing RosaControlPlan API fields;
Tags,
EtcdEncryption,
EtcdEncryptionKMSArn
Also removing the accountID and creatorARN from ROSA cluster templates as it is not necessary to declare them explicitly in the rosaControlPlane spec along the recent change.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes # #4737
Special notes for your reviewer:
There is another PR for adding different fields to RosaControlplan delivered by @serngawy
#4780
I will rebase the PR after the 4780 PR is merged.
Checklist:
Release note: