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

Graduate APIVersion to elbv2.k8s.aws/v1 #3916

Open
M00nF1sh opened this issue Oct 28, 2024 · 0 comments
Open

Graduate APIVersion to elbv2.k8s.aws/v1 #3916

M00nF1sh opened this issue Oct 28, 2024 · 0 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@M00nF1sh
Copy link
Collaborator

M00nF1sh commented Oct 28, 2024

We have been stuck with v1beta version for our CRDs for a long time, i think it's time to graduate it to v1.
Creating this PR to track a few actions items before we can use v1:

  1. We should have a workflow that checks all CRDs fields adheres to k8s API convention
    • e.g. check optional fields have both "+optional" and "omitempty"
    • e.g. check optional fields are pointer type except when it's default value is nil (slice/map)
  2. In our PR template, ask authors to read https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md before adding new CRD fields
  3. Add a conversion Webhook to facilitate API changes
  4. Remove the v1alpha1 API
  5. TODO API changes for v1:
    1. rename IngressClassParams.spec.listeners.listenerAttributes to be IngressClassParams.spec.listeners.attributes
      • Unlike the 'loadBalancerAttributes', the 'listenerAttributes' are already configured on 'listeners'.
        This approach is straightforward and aligns with the Kubernetes API convention."
      • It matches ELBv2 ModifyListenerAttributes where it's simply 'attributes' in ModifyListenerAttributes
    2. rename IngressClassParams.spec.certificateArn to be IngressClassParams.spec.certificateARNs
      • It reflects that it's a list instead of a single item
      • It aligns with k8s api convention(use the same case for abbreviation ARN), and aligns with targetGroupARN in TGB API.
    3. change IngressClassParams.spec.subnets.tags to use structured fields instead of map[string][string]
    1. remove the "vpcID" & "ipAddressType" from the "TargetGroupBinding.spec" CRD.
      • Those "TargetGroup" specific attributes shall be obtained during TGB reconciliation instead of persisted as part of TGB spec.
      • We cannot keep adding new "TargetGroup" specific attributes into TGB spec in order to use them during reconcile, it's not scalable.
    2. consider to change "TargetGroupBinding.spec.targetType" to be required instead of "optional".
      * We can avoid doing elbv2 api calls as part of webhook.
      * The targetType in theory is not same as targetType of targetGroup. (e.g. ideally we can register node ports targets into targetGroup of ip target type)
@shraddhabang shraddhabang added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

2 participants