feat: provider generic annotations

[farodin91]

Description With the new webhook functionality, it is possible to add provider specific annotations for webhook provider at moment.

This solves it in a generic way without changing anything.

Checklist

• [x] Unit tests updated
• [ ] End user documentation updated

[k8s-ci-robot]

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

[farodin91]

@Raffo I tried to fix provider specific annotations in generic way as possible.

[johngmyers]

I'm not sure "without changing anything" should be a design goal.

I think we're going to need to have providers filter such annotations. AdjustEndpoints could do it, but if the provider takes no explicit action then any such annotation will cause the endpoint to be handed to ApplyChanges on every sync.

The vast majority of providers don't have any provider-specific annotations and these should filter them all out. We could make BaseProvider.AdjustEndpoints() filter out all provider-specific annotations, perhaps.

[farodin91]

@johngmyers I added a filter out for generic annotation without filtering out old ProviderSpecific stuff.

[johngmyers]

I hadn't planned on tackling the problem of provider-specific annotations quite yet, but since someone's got the desire to work on them right now I should write down my thoughts:

• Knowledge of what the provider-specific annotations are should not be in core (as it currently is) but in the respective providers.
  • This will require extending the provider API to permit the provider to map annotations to provider-specific properties.
  • Knowledge of the existing provider-specific annotations will need to be pushed into their respective providers. A corollary of this is that the base provider shouldn't treat them specially.
  • Some existing provider-specific annotations might be core. set-identifier is because it has to be handled in core. alias might make sense for some other non-AWS provider
  • So that the webhook doesn't have to be called for each resource, this might be a call-upon-startup method returning a map from annotation key to provider-specific property key.
• We will want to introduce stable annotations, not under the alpha.kubernetes.io. (I plan a project to graduate several annotations to stable.)
  • Stable versions of provider-specific annotations might belong under a domain belonging to the provider. On the other hand, there are providers maintained by people not affiliated by the services they talk to.

[farodin91]

@johngmyers I will look into this next week.

[farodin91]

@johngmyers I started to look into the implementation.

[farodin91]

@johngmyers My first idea based on your feedback.

• webhook backwards combat
• move everything to provider
• tried to make as generic as possible

ToDo:

• tests

[farodin91]

/assign @johngmyers

[mloiseleur]

@farodin91 This PR is quite interesting. Do you think you can rebase it ?

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from johngmyers. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[farodin91]

@mloiseleur Updated, if you like to give feedback and go for it.

[mloiseleur]

/ok-to-test

[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

[farodin91]

@mloiseleur Would you like to review?

[farodin91]

@Raffo https://github.com/kubernetes-sigs/external-dns/pull/4458

[k8s-ci-robot]

PR needs rebase.

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-sigs/prow repository.

[Raffo]

@farodin91 I totally missed this PR, sorry. Can you walk me through your changes here and how you'd like to proceed with respect to #4458 ?

[farodin91]

@farodin91 I totally missed this PR, sorry. Can you walk me through your changes here and how you'd like to proceed with respect to #4458 ?

My change allows every provider to easily provide their own set of annotations. For me it was also important to support every existing custom annotation. If you think your PR get merged, I'm fine with it as it can solve my issue. Also, each Webhook provider can provide their own set own annotations.

[Raffo]

We shipped https://github.com/kubernetes-sigs/external-dns/pull/4458, so I'm closing this PR.