helm: add Dex wrapper chart#3480
Conversation
|
Welcome to the Kubeflow Manifests Repository Thanks for opening your first PR. Your contribution means a lot to the Kubeflow community. Before making more PRs: Community Resources:
Thanks again for helping to improve Kubeflow. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Dex to the Helm↔Kustomize comparison harness and introduces an initial Dex Helm chart intended to render parity manifests with common/dex.
Changes:
- Extend the compare scripts to support the new
dexcomponent and scenarios. - Add a new experimental Helm chart for Dex, including templates, CRD, and CI values files.
- Adjust manifest-key normalization to include namespaces for Dex comparisons.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/helm_kustomize_compare_all.sh | Adds dex scenarios and CLI help output for running comparisons across all components. |
| tests/helm_kustomize_compare.sh | Adds Dex chart/manifests paths and Helm templating logic for Dex scenarios. |
| tests/helm_kustomize_compare.py | Updates component allowlist and resource-keying behavior to support Dex. |
| experimental/helm/charts/dex/Chart.yaml | Introduces the Dex Helm chart metadata. |
| experimental/helm/charts/dex/values.yaml | Adds default chart values for namespaces, Dex config, and credentials. |
| experimental/helm/charts/dex/templates/_helpers.tpl | Adds namespace helper templates used by Dex resources. |
| experimental/helm/charts/dex/templates/dex.yaml | Adds Dex workload resources (SA, ConfigMap, Secrets, Service, Deployment). |
| experimental/helm/charts/dex/templates/rbac.yaml | Adds ClusterRole/ClusterRoleBinding for Dex. |
| experimental/helm/charts/dex/templates/namespace.yaml | Adds optional Namespace rendering for Dex workload namespace. |
| experimental/helm/charts/dex/templates/istio.yaml | Adds optional Istio VirtualService for routing to Dex. |
| experimental/helm/charts/dex/templates/networkpolicies.yaml | Adds optional NetworkPolicies for Dex ingress. |
| experimental/helm/charts/dex/crds/authcodes.yaml | Adds Dex AuthCode CRD (installed via Helm CRDs mechanism). |
| experimental/helm/charts/dex/ci/values-*.yaml | Adds scenario-specific values files for parity comparisons. |
| experimental/helm/charts/dex/README.md | Documents install/caveats and comparison commands for the Dex chart. |
| id: kubeflow-oidc-authservice | ||
| # -- OAuth client secret used by oauth2-proxy. | ||
| secret: pUBnBOY80SnXgjibTYM9ZWNzY2xreNGQok | ||
|
|
||
| staticPassword: | ||
| # -- Bcrypt hash for the default Dex static user password. | ||
| hash: $2y$12$4K/VkmDd1q1Orb3xAt82zu8gk7Ad6ReFR4LCP9UeYE90NLiN9Df72 |
There was a problem hiding this comment.
Moved the Kustomize parity secrets into CI values and left placeholders in chart defaults.
| data: | ||
| OIDC_CLIENT_ID: {{ .Values.oidcClient.id | b64enc }} | ||
| OIDC_CLIENT_SECRET: {{ .Values.oidcClient.secret | b64enc }} |
There was a problem hiding this comment.
Kept data for dex-oidc-client to preserve Kustomize secretGenerator parity; dex-passwords already uses stringData like Kustomize.
| kind: Namespace | ||
| metadata: | ||
| labels: | ||
| pod-security.kubernetes.io/enforce: restricted |
There was a problem hiding this comment.
Made namespace labels configurable while keeping the current restricted Kustomize default.
| web: | ||
| http: 0.0.0.0:5556 | ||
| logger: | ||
| level: "debug" |
There was a problem hiding this comment.
Made Dex log level configurable; chart defaults to info, while CI parity values keep Kustomize debug.
| # Include namespace in key for components that render same-name resources | ||
| # across release/workload namespaces or multiple workload namespaces. | ||
| if component in ["katib", "dex"] and namespace: |
There was a problem hiding this comment.
Fixed by using namespace-aware resource keys whenever a manifest has metadata.namespace, instead of a Dex-specific allowlist.
41f9898 to
a847509
Compare
|
Please first update kustomize to the latest available version in a separate PR. |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
bf386ad to
f3ce9a7
Compare
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
|
Addressed: Dex Kustomize manifests were synchronized to |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
There was a problem hiding this comment.
why do we have this file? is it supposed to be meaningfully used standalone? What is the customer scenario?
juliusvonkohout
left a comment
There was a problem hiding this comment.
ad script in /scripts to automatically update the kustomize and helm for dex.
There was a problem hiding this comment.
what is the purpose of this file? do you intent do deploy dex without oauth2-proxy ? what is the customer scenario ?
There was a problem hiding this comment.
i want one meainingful value file, not 3
|
and please move this to common/dex/helm. |
|
each extra line costs maintenance, keep this in mind. |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
|
[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 |
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Signed-off-by: danish9039 <danishsiddiqui040@gmail.com>
Summary
Adds the Dex Helm wrapper chart for the Project 5 platform-first Helm work under
common/dex/helm, co-located with the Kustomize baseline undercommon/dex.The chart is a static Kustomize-parity wrapper for the current Kubeflow Dex install. It follows
common/dexand is aligned with Dexv2.45.1.Supported comparison scenarios
base:common/dex/baseoverlays-istio:common/dex/overlays/istioScope update
experimental/helm/charts/dextocommon/dex/helm.common/*/helm/templates/**are not treated as raw YAML.tests/helm_kustomize_compare.shwith the co-located chart path andbase/overlays-istioscenarios.Follow-up
Sync script extension for appVersion, image tag, and CRD updates will follow separately after the chart location change, so this PR stays focused on co-location and parity proof.
Stacking note
This PR is part of the platform auth chain and follows the foundation, cert-manager, Istio, and oauth2-proxy wrapper work:
The chart does not render
Namespace/auth. That namespace is provided by the foundation chart in #3468, and the Helm release metadata is stored in theauthworkload namespace.Values note
The chart keeps Kustomize parity defaults in the CI values files. Production installs should provide their own static users, OIDC client values, and password hashes.
oidcClient.idandoidcClient.secretare raw values and are encoded by the template.Validation