WIP: Add Application Load Balancer Controller Manager#879
WIP: Add Application Load Balancer Controller Manager#879kamilprzybyl wants to merge 20 commits intomainfrom
Conversation
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ske-prow
bot
added
do-not-merge/needs-kind
kamilprzybyl
changed the title
ske-prow
bot
added
the
do-not-merge/work-in-progress
kamilprzybyl
force-pushed
the
feat/kp/add-alb-ingress-controller
branch
from
7ecc416 to
86bdf1d
Compare
Added ExactMatch -Path- to change detection
| // generate self-signed certificates for the metrics server. While convenient for development and testing, | ||
| // this setup is not recommended for production. | ||
| // | ||
| // TODO(user): If you enable certManager, uncomment the following lines: |
There was a problem hiding this comment.
might be good to create separate examples for that
| certOpts = append(certOpts, sdkconfig.WithEndpoint(certURL)) | ||
| } | ||
|
|
||
| fmt.Printf("Create ALB SDK client\n") |
There was a problem hiding this comment.
please remove fmt.Printf or use setupLog instead
| ) | ||
|
|
||
| const ( | ||
| // externalIPAnnotation references an OpenStack floating IP that should be used by the application load balancer. |
There was a problem hiding this comment.
do we want to reference openstack here as its deprecated?
| if err != nil { | ||
| log.Printf("failed to load tls certificates: %v", err) | ||
| //nolint:gocritic // TODO: Rework error handling. | ||
| // return nil, fmt.Errorf("failed to load tls certificates: %w", err) |
| sort.SliceStable(ruleMetadataList, func(i, j int) bool { | ||
| a, b := ruleMetadataList[i], ruleMetadataList[j] | ||
| // 1. Host name (lexicographically) | ||
| if a.host != b.host { |
| } | ||
|
|
||
| // cleanupCerts deletes the certificates from the Certificates API that are no longer associated with any Ingress in the IngressClass | ||
| func (r *IngressClassReconciler) cleanupCerts(ctx context.Context, ingressClass *networkingv1.IngressClass, ingresses []*networkingv1.Ingress) error { |
There was a problem hiding this comment.
do we really want to delete certificates? That could be very dangerous as users might just not associate the certificate anymore and want to keep it.
|
|
||
| // isCertValid checks if the certificate chain is complete. It is used for checking if | ||
| // the cert-manager's ACME challenge is completed, or if it's sill ongoing. | ||
| func isCertValid(secret *corev1.Secret) (bool, error) { |
There was a problem hiding this comment.
isn't that already handled by the certificate api?
| acme: | ||
| server: https://acme-v02.api.letsencrypt.org/directory | ||
| # server: https://acme-staging-v02.api.letsencrypt.org/directory | ||
| email: kamil.przybyl@stackit.cloud |
There was a problem hiding this comment.
don't use a developers mail address in the examples
ske-prow
bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions 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. |
| } | ||
|
|
||
| if len(albIngressList) < 1 { | ||
| err := r.handleIngressClassWithoutIngresses(ctx, albIngressList, ingressClass) |
There was a problem hiding this comment.
It doesn't make sense to pass albIngressList if it is always empty.
4 participants
How to categorize this PR?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Breaking changes: