Validate additionalTags

[jonathanbeber]

What type of PR is this?

/kind bug

What this PR does / why we need it:

As described in #3102, tags keys currently can be empty or do not
respect other AWS rules for tagging, resulting in errors during
resources provisioning.

This commit adds validation to the tags in the webhook, for all the
resource where additionalTags is present, during creation and update.

Validation is done in the webhook instead of OpenAPI schema due to
limitations in the current additionalTags field type, as better
described in #3102.

Which issue(s) this PR fixes:
Part of #3102

Special notes for your reviewer:

Checklist:

• squashed commits
• adds unit tests
• adds or updates e2e tests

Release note:

additionalTags are now validated to comply with AWS API requirements

[k8s-ci-robot]

@jonathanbeber: This issue is currently awaiting triage.

If CAPA/CAPI contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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]

Hi @jonathanbeber. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[sedefsavas]

/ok-to-test

[sedefsavas]

LGTM.

One comment: is it enough to add just a single test as tags_test.go, given that same tag validation is used by all webhooks?

[richardcase]

Thanks @jonathanbeber

We also use tags in a number of other resource kinds:

• AWSManagedControlPlane
• AWSFargateProfile
• AWSMachinePool
• AWSManagedMachinePool

Do you think it would be possible to add similar validation checks to the webhooks for these? This would be great as it would give us consistent tag validation across all our resource kinds 🎉

[jonathanbeber]

Thank you very much for your quick review and feedback.

LGTM.

One comment: is it enough to add just a single test as tags_test.go, given that same tag validation is used by all webhooks?

Maybe I didn't follow completely this one, @sedefsavas. You mean like testing in just one of the webhooks? That would leave us in a situation where a line like this one is removed and the tests wouldn't break.

---

@richardcase sure thing, I'm not sure how I missed those 🤦‍♂️ but for sure I will work in adding them too.

[jonathanbeber]

@richardcase I added the validation for other resources. I realised I ignored them because they were part of other packages, thanks for pointing it out.

For some of the resources I had to add new test cases as the validation was not being tested.

[sedefsavas]

/lgtm

/assign @richardcase

[richardcase]

Thanks for making those changes @jonathanbeber .

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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:

• OWNERS [richardcase]

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