WIP: Configurable load balancer attributes (access logging and read timeout) #218
Conversation
There was a problem hiding this comment.
@antoniobeyah initial annotation work looks good. Small change requested. Will review again once you propagate this to ALB create / modification.
Thanks for the work!
| switch { | ||
| case *rawAttr == "": | ||
| continue | ||
| case len(parts) < 2: |
There was a problem hiding this comment.
You'll always expect parts to have length 2 here, correct?
Possibly best to set this check such that when it's anything other than 2, we return the error. Preventing cases like attr=value=value2
|
@joshrosso how should the controller deal with configuration items there were set outside of the controller? Looking at the way the reconcile works if someone setup access logging outside the scope of the controller and did not add in the matching annotations access logging would be disabled. Is this something we should try to handle in code or just add it as a warning in the changelog? |
|
@antoniobeyah Our approach thus far has been that all ALBs the controller manages always uses the desired state (derived from ingress manifests) as what we'll eventually get to. Meaning, any configuration options made outside of the ALB (ie AWS) should be overwritten by the controller in the next reconcile. Let me know if this answers your Q. |
|
Hi, Because we needed to setup the ALB access log, I finished the work started by @antoniobeyah in this PR. Since it is not possible to contribute directly to this PR, I've open a new one: #279 . I've tested the feature on a live k8s cluster. Please let me know if there's anything I need to do. |
Fixes #90 (access logging) and #79 (read timeout)