feat: make the webhook port configurable #301
Conversation
Signed-off-by: Argannor <[email protected]>
Argannor
force-pushed
the
configurable-port
branch
from
2d2306f to
3fb096f
Compare
| @@ -63,6 +63,7 @@ func main() { | |||
| leaderElection = app.Flag("leader-election", "Use leader election for the controller manager.").Short('l').Default("false").Envar("LEADER_ELECTION").Bool() | |||
| maxReconcileRate = app.Flag("max-reconcile-rate", "The number of concurrent reconciliations that may be running at one time.").Default("100").Int() | |||
| sanitizeSecrets = app.Flag("sanitize-secrets", "when enabled, redacts Secret data from Object status").Default("false").Envar("SANITIZE_SECRETS").Bool() | |||
| webhookPort = app.Flag("webhook-port", "The port the webhook listens on").Default("9443").Envar("WEBHOOK_PORT").Int() | |||
There was a problem hiding this comment.
for all providers, also allow configuring metrics bind address which gets passed onto sigs.k8s.io/controller-runtime/pkg/metrics/server.Options ?
we can standardize on --webhook-port and --metrics-bind-address flags across all providers, similar to crossplane core flags..
There was a problem hiding this comment.
+1 for standardizing. I added the --metrics-bind-address as well as requested. Thank you for the feedback!
There was a problem hiding this comment.
by the time we finished crossplane/crossplane#5540, we ended up with:
--webhook-port=9443 The port the webhook server listens on ($WEBHOOK_PORT).
--metrics-port=8080 The port the metrics server listens on ($METRICS_PORT).
So we should update this PR to those values too, so we keep with the intended standardization. Let's try not ship a version of this provider and then change the flag later on 🤓
@Argannor are you still up for driving this PR to completion?
There was a problem hiding this comment.
@jbw976 please see crossplane/crossplane#5540 (comment)
the argument towards keeping the metrics port configuration as address string is: Some Security focussed setups would not want to expose the metrics port on all interfaces, instead only on 127.0.0.1:8080 and then front it with a rbac-proxy sidecar which requires auth for the prometheus scrapers to be able to reach it.
Intaking it as metrics-port int arg, will take away above ability.
the webhook port being passed in as int arg is fine because, the webhook server is TLS enabled and requires the client(Kuberntes API Server) to have access to self-signed CA for verification.
Signed-off-by: Argannor <[email protected]>
|
I could really use this sooner rather than later. @ravilr is there anyway this can be prioritized? Really appreciate your work either way. |
|
What is stopping this from being merged? Thanks for the work by the way. |
jeanduplessis
requested review from
turkenh,
morningspace,
lsviben,
phisco,
sergenyalcin and
turkenf
as code owners
Description of your changes
As outlined in the issue below, when running Crossplane in certain environments (like EKS using Calico CNI), where the control plane resides outside the workload network it is necessary to run webhooks in the host network. This calls for the need to make the port configurable to avoid conflicts.
I opened crossplane/crossplane!5540 to allow a
DeploymentRuntimeConfigto overwrite the ports of the crossplane managed services created for each provider / function.Fixes #208
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
I deployed my fork of this and crossplane in our test cluster running EKS and Calico. With this I was able to run both Crossplane and provider-kubernetes inside the host network with custom ports.
This change only makes sense if my other PR gets accepted.