Update Dex dependency#279
Conversation
|
need rebase. otherise lgtm |
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughVersion updates to dex and gRPC, extensive go.mod replace adjustments aligning Kubernetes and multicluster-runtime dependencies, documentation formatting and terminology normalization, a quickstart doc update adding a dex client secret, and minor comment casing changes in dev configs and Kind setup script. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
docs/content/setup/quickstart.md (1)
35-42: Avoid committing a real-looking client secret in docs; use a placeholder.
Replace with a clearly fake<CLIENT_SECRET>to prevent gitleaks noise and copy-paste to prod.Apply:
- secret: ZXhhbXBsZS1hcHAtc2VjcmV0 + # Example only. Generate your own and keep it secret. + secret: <CLIENT_SECRET>docs/content/setup/local-setup-with-kind.md (1)
90-98: Doc embeds a static client secret; prefer placeholder to reduce leak-noise.
Same suggestion as quickstart to avoid gitleaks false positives.- secret: ZXhhbXBsZS1hcHAtc2VjcmV0 + secret: <CLIENT_SECRET> # generate your own for local runsgo.mod (1)
13-29: Scope the “Matches multicluster-runtime v0.21.0-alpha.8” block.The block does more than align to multicluster-runtime: it also forces CEL and multiple k8s modules. Consider narrowing it to only what multicluster-runtime truly needs, and let the rest resolve via require to reduce churn.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
cli/go.sumis excluded by!**/*.sumdocs/generators/cli-doc/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
Makefile(2 hunks)cli/go.mod(1 hunks)docs/content/setup/local-setup-with-kind.md(5 hunks)docs/content/setup/quickstart.md(4 hunks)docs/generators/cli-doc/go.mod(1 hunks)go.mod(2 hunks)hack/dex-config-dev.yaml(1 hunks)hack/kind-example-setup/backend-cluster.sh(1 hunks)kcp/dex-config-dev.yaml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
docs/content/setup/quickstart.md
[high] 82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Generate and push docs
- GitHub Check: lint
- GitHub Check: verify
- GitHub Check: go-test
- GitHub Check: go-test-e2e
🔇 Additional comments (8)
hack/kind-example-setup/backend-cluster.sh (1)
41-41: Nit: casing change is fine.
No functional impact.Makefile (2)
166-166: Minor: tools target whitespace cleanup.
No behavior change.
88-91: Dex toolchain bump — tag exists; run e2e to confirm compatibility.
refs/tags/v2.43.1 exists; Makefile sets DEX_VER := v2.43.1 (Makefile lines 88–91) and calls$(DEX) serve hack/dex-config-dev.yaml(line 274). E2E cannot be executed in this environment — runmake test-e2e(or start$(DEX) serve hack/dex-config-dev.yaml) locally/CI against v2.43.1 to confirm the upstream tag provides a compatible binary/Makefile and that the e2e config still works.docs/generators/cli-doc/go.mod (1)
109-109: grpc alignment with workspace.
Matches other modules; keep them in lockstep going forward.hack/dex-config-dev.yaml (1)
96-98: Comment casing only.
No functional change; safe.kcp/dex-config-dev.yaml (1)
96-98: Comment casing only.
No functional change; safe.docs/content/setup/local-setup-with-kind.md (1)
62-66: Comment casing change is fine.
No behavior change.go.mod (1)
33-34: Confirm Dex API version bump — PR text says v2.1.0 but repo uses v2.3.0.Evidence: root go.mod lists github.com/dexidp/dex/api/v2 v2.3.0; import found at test/e2e/framework/backend.go:27. Action: confirm the intended target version and server/gRPC compatibility; if v2.3.0 is correct, update the PR description and release notes, otherwise revert/update go.mod and regenerate any affected clients/tests.
| google.golang.org/genproto/googleapis/api v0.0.0-20250106144421-5f5ef82da422 // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20250106144421-5f5ef82da422 // indirect | ||
| google.golang.org/grpc v1.69.2 // indirect | ||
| google.golang.org/grpc v1.70.0 // indirect |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
grpc bump looks fine; verify single-version across modules.
Consistent with root and docs mods. Please run tidy across all modules and confirm a single picked grpc version.
🏁 Script executed:
#!/bin/bash
# Ensure all go.mod pick the same grpc version and are tidy
fd -H go.mod | while read -r mod; do
(cd "$(dirname "$mod")" && go mod tidy -v >/dev/null 2>&1 || true && go list -m -f '{{.Path}} {{.Version}}' all | rg -n '^google.golang.org/grpc ' -n) | sed "s|^|$(dirname "$mod"): |"
doneLength of output: 990
Unify google.golang.org/grpc version across modules
- Script output shows mixed grpc versions: cli + root = v1.70.0; kcp = v1.69.2; sdk/apis & sdk/client = v1.65.0.
- go mod tidy also failed with "unknown revision v0.0.0" for several k8s.io modules — resolve those (correct versions or add replaces) so tidy can complete.
- Action: run go mod tidy in kcp, sdk/apis, sdk/client (and root/cli), align all modules to a single grpc version (recommend root's v1.70.0 or your chosen version), commit updated go.mod/go.sum, then re-run the verification script to confirm a single picked grpc version.
🤖 Prompt for AI Agents
In cli/go.mod around line 117, multiple modules use different
google.golang.org/grpc versions and go mod tidy fails due to k8s.io modules
referencing unknown revision v0.0.0; to fix, run go mod tidy in kcp, sdk/apis,
sdk/client, root and cli to surface errors, correct the k8s.io module versions
(or add explicit replace directives pointing to valid tags/commits) so tidy
completes, align all modules to a single grpc version (add/adjust require or
replace entries to pin google.golang.org/grpc to v1.70.0 across modules), commit
the updated go.mod and go.sum files, then re-run the verification script to
confirm only one grpc version is picked.
| * start the backend binary with the right flags: | ||
| ```shell | ||
| make build | ||
| bin/backend \ | ||
| --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \ | ||
| --oidc-issuer-client-id=kube-bind \ | ||
| --oidc-issuer-url=http://127.0.0.1:5556/dex \ | ||
| --oidc-callback-url=http://127.0.0.1:8080/callback \ | ||
| --pretty-name="BigCorp.com" \ | ||
| --namespace-prefix="kube-bind-" \ | ||
| --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \ | ||
| --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \ | ||
| --consumer-scope=cluster | ||
| ``` | ||
| ```shell | ||
| make build | ||
|
|
||
| bin/backend \ | ||
| --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \ | ||
| --oidc-issuer-client-id=kube-bind \ | ||
| --oidc-issuer-url=http://127.0.0.1:5556/dex \ | ||
| --oidc-callback-url=http://127.0.0.1:8080/callback \ | ||
| --pretty-name="BigCorp.com" \ | ||
| --namespace-prefix="kube-bind-" \ | ||
| --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \ | ||
| --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \ | ||
| --consumer-scope=cluster | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Example cookie keys trigger secret scanners; use placeholders.
These static values are flagged by gitleaks. Prefer placeholders and show how to generate them.
Apply:
- --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
- --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+ --cookie-signing-key=<BASE64_32_OR_64_BYTES> \
+ --cookie-encryption-key=<BASE64_16_24_OR_32_BYTES> \Optionally add a note right below with openssl rand -base64 32.
🏁 Script executed:
#!/bin/bash
# Show any committed hard-coded demo secrets to update
rg -n 'ZXhhbXBsZS1hcHAtc2VjcmV0|cookie-(signing|encryption)-key=' docs -nLength of output: 948
Replace demo secrets in docs with placeholders (triggers gitleaks)
Static demo secrets (OIDC client secret, cookie signing/encryption keys) are present and flagged by secret scanners; replace them with placeholders and show how to generate strong values.
Occurrences to update:
- docs/content/setup/quickstart.md — lines 41, 76, 82–83, 87
- docs/content/setup/local-setup-with-kind.md — lines 96, 169, 175–176
Apply:
- secret: ZXhhbXBsZS1hcHAtc2VjcmV0
+ secret: <OIDC_CLIENT_SECRET>
...
- --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \
+ --oidc-issuer-client-secret=<OIDC_CLIENT_SECRET> \
- --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \
- --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \
+ --cookie-signing-key=<BASE64_32_OR_64_BYTES> \
+ --cookie-encryption-key=<BASE64_16_24_OR_32_BYTES> \Add a one-line note showing generation commands, e.g.
- openssl rand -base64 32 (generate 32 random bytes)
- openssl rand -base64 16|24|32 (use 16/24/32 bytes for AES keys)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * start the backend binary with the right flags: | |
| ```shell | |
| make build | |
| bin/backend \ | |
| --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \ | |
| --oidc-issuer-client-id=kube-bind \ | |
| --oidc-issuer-url=http://127.0.0.1:5556/dex \ | |
| --oidc-callback-url=http://127.0.0.1:8080/callback \ | |
| --pretty-name="BigCorp.com" \ | |
| --namespace-prefix="kube-bind-" \ | |
| --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \ | |
| --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \ | |
| --consumer-scope=cluster | |
| ``` | |
| ```shell | |
| make build | |
| bin/backend \ | |
| --oidc-issuer-client-secret=ZXhhbXBsZS1hcHAtc2VjcmV0 \ | |
| --oidc-issuer-client-id=kube-bind \ | |
| --oidc-issuer-url=http://127.0.0.1:5556/dex \ | |
| --oidc-callback-url=http://127.0.0.1:8080/callback \ | |
| --pretty-name="BigCorp.com" \ | |
| --namespace-prefix="kube-bind-" \ | |
| --cookie-signing-key=bGMHz7SR9XcI9JdDB68VmjQErrjbrAR9JdVqjAOKHzE= \ | |
| --cookie-encryption-key=wadqi4u+w0bqnSrVFtM38Pz2ykYVIeeadhzT34XlC1Y= \ | |
| --consumer-scope=cluster | |
| ``` | |
| * start the backend binary with the right flags: |
🧰 Tools
🪛 Gitleaks (8.27.2)
[high] 82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In docs/content/setup/quickstart.md around lines 71 to 85 (and also update the
other occurrences listed: lines 41, 76, 82–83, 87 in this file and
docs/content/setup/local-setup-with-kind.md lines 96, 169, 175–176), remove the
hard-coded demo secrets and replace them with clear placeholders (e.g.
<OIDC_CLIENT_SECRET>, <COOKIE_SIGNING_KEY>, <COOKIE_ENCRYPTION_KEY>) in the
example backend command; add a single-line note near the example showing how to
generate strong values (suggest commands such as openssl rand -base64 32 and
openssl rand -base64 16|24|32) and mention which sizes to use for AES keys so
readers can generate secure keys instead of copying demo secrets.
| // Matches multicluster-runtime v0.21.0-alpha.8 | ||
| replace ( | ||
| github.com/google/cel-go => github.com/google/cel-go v0.22.0 | ||
| github.com/kcp-dev/apimachinery/v2 => github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250728122101-adbf20db3e51 | ||
| github.com/kcp-dev/multicluster-provider => github.com/mjudeikis/kcp-multicluster-provider v0.0.0-20250818102159-3d31cbb06ebe | ||
| k8s.io/api => k8s.io/api v0.33.0 | ||
| k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.33.0 | ||
| k8s.io/apimachinery => k8s.io/apimachinery v0.33.0 | ||
| k8s.io/apiserver => k8s.io/apiserver v0.33.0 | ||
| k8s.io/cli-runtime => k8s.io/cli-runtime v0.33.0 | ||
| k8s.io/client-go => k8s.io/client-go v0.33.0 | ||
| k8s.io/code-generator => k8s.io/code-generator v0.33.0 | ||
| k8s.io/component-base => k8s.io/component-base v0.33.0 | ||
| sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.21.0 | ||
| //sigs.k8s.io/multicluster-runtime => sigs.k8s.io/multicluster-runtime v0.20.4-alpha.7 | ||
| sigs.k8s.io/multicluster-runtime => github.com/mjudeikis/sigs-multicluster-runtime v0.0.0-20250818101434-d8ebc45e169b | ||
| ) |
There was a problem hiding this comment.
Avoid version skew: replace downgrades K8s and cel-go below required versions.
Your replace block pins k8s.io/* to v0.33.0 while require uses v0.33.3, and forces cel-go v0.22.0 while indirect requires v0.23.2. This will cause subtle ABI drift and harder-to-debug build issues. Align them.
Apply this diff to match the require set:
replace (
- github.com/google/cel-go => github.com/google/cel-go v0.22.0
+ github.com/google/cel-go => github.com/google/cel-go v0.23.2
github.com/kcp-dev/apimachinery/v2 => github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250728122101-adbf20db3e51
github.com/kcp-dev/multicluster-provider => github.com/mjudeikis/kcp-multicluster-provider v0.0.0-20250818102159-3d31cbb06ebe
- k8s.io/api => k8s.io/api v0.33.0
- k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.33.0
- k8s.io/apimachinery => k8s.io/apimachinery v0.33.0
- k8s.io/apiserver => k8s.io/apiserver v0.33.0
- k8s.io/cli-runtime => k8s.io/cli-runtime v0.33.0
- k8s.io/client-go => k8s.io/client-go v0.33.0
- k8s.io/code-generator => k8s.io/code-generator v0.33.0
- k8s.io/component-base => k8s.io/component-base v0.33.0
+ k8s.io/api => k8s.io/api v0.33.3
+ k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.33.0
+ k8s.io/apimachinery => k8s.io/apimachinery v0.33.3
+ k8s.io/apiserver => k8s.io/apiserver v0.33.0
+ k8s.io/cli-runtime => k8s.io/cli-runtime v0.32.0
+ k8s.io/client-go => k8s.io/client-go v0.33.3
+ k8s.io/code-generator => k8s.io/code-generator v0.33.0
+ k8s.io/component-base => k8s.io/component-base v0.33.0
)Also applies to: 54-61, 102-103
🤖 Prompt for AI Agents
In go.mod around lines 13 to 29, the replace block pins k8s.io/* modules to
v0.33.0 and github.com/google/cel-go to v0.22.0 which downgrades them relative
to the required versions and can cause ABI/version skew; update the replace
entries to match the require set (e.g., set k8s.io/api, apimachinery, apiserver,
client-go, cli-runtime, code-generator, component-base, apiextensions-apiserver
to v0.33.3 and set github.com/google/cel-go to v0.23.2) and make the same
alignment for the other mentioned ranges (lines 54-61 and 102-103) so replaces
no longer force older versions than the module requires.
| github.com/kcp-dev/multicluster-provider => github.com/mjudeikis/kcp-multicluster-provider v0.0.0-20250818102159-3d31cbb06ebe | ||
| k8s.io/api => k8s.io/api v0.33.0 |
There was a problem hiding this comment.
Don’t ship with personal forks in replace.
Both multicluster-provider and multicluster-runtime are redirected to personal forks. That’s a supply‑chain and long‑term maintenance risk. Revert to upstream or justify with a temporary branch and follow‑up issue to remove before release.
Apply this diff (unless there’s a blocking upstream bug and an issue is linked):
replace (
github.com/google/cel-go => github.com/google/cel-go v0.23.2
github.com/kcp-dev/apimachinery/v2 => github.com/kcp-dev/apimachinery/v2 v2.0.1-0.20250728122101-adbf20db3e51
- github.com/kcp-dev/multicluster-provider => github.com/mjudeikis/kcp-multicluster-provider v0.0.0-20250818102159-3d31cbb06ebe
+ # keep upstream unless absolutely necessary
+ # github.com/kcp-dev/multicluster-provider => github.com/mjudeikis/kcp-multicluster-provider v0.0.0-20250818102159-3d31cbb06ebe
…
- //sigs.k8s.io/multicluster-runtime => sigs.k8s.io/multicluster-runtime v0.20.4-alpha.7
- sigs.k8s.io/multicluster-runtime => github.com/mjudeikis/sigs-multicluster-runtime v0.0.0-20250818101434-d8ebc45e169b
+ # sigs.k8s.io/multicluster-runtime => github.com/mjudeikis/sigs-multicluster-runtime v0.0.0-20250818101434-d8ebc45e169b
)If forks are required, add a comment with the upstream issue/PR and a TODO to revert.
Also applies to: 27-29
🤖 Prompt for AI Agents
In go.mod around lines 17-18 (and also check related replaces at lines 27-29),
the file contains replace directives pointing to personal forks
(github.com/mjudeikis/*), which must not be shipped; either revert these replace
directives to the upstream modules (remove or change them to the official module
paths/versions) or, if a fork is temporarily required due to a blocking upstream
bug, add an explicit comment above each replace linking the upstream issue/PR
and a TODO to remove the fork before release; update the go.mod accordingly and
ensure a follow-up issue is created and referenced in the comment.
| github.com/vmihailenco/msgpack/v4 v4.3.12 | ||
| golang.org/x/oauth2 v0.29.0 | ||
| google.golang.org/grpc v1.69.2 | ||
| google.golang.org/grpc v1.70.0 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
gRPC 1.70.0: check transitive consistency (genproto/protobuf) and duplicate versions.
Avoid mixed gRPC trees across submodules. Ensure genproto/protobuf versions are compatible and that cli/ and docs/ modules match.
Run:
If mismatches exist, align all go.mod files and re‑tidy.
🏁 Script executed:
#!/bin/bash
# quick consistency check across go.mod files
fd -a go.mod | xargs -I{} sh -c "echo '--- {}'; sed -n '/google.golang.org\\/grpc/p' {} ; sed -n '/google.golang.org\\/protobuf/p' {}; sed -n '/google.golang.org\\/genproto/p' {}"Length of output: 1818
Align gRPC / genproto / protobuf versions across modules
Detected concrete mismatches — action required:
- kcp/go.mod has google.golang.org/grpc v1.69.2 while root, cli, and docs use v1.70.0.
- sdk/client/go.mod has google.golang.org/protobuf v1.35.1 while others use v1.36.5.
- root go.mod lists google.golang.org/genproto v0.0.0-202402... while cli/docs/kcp reference google.golang.org/genproto/googleapis/* at 202501... (inconsistent genproto entries).
Fix: pick canonical versions (e.g., grpc v1.70.0, protobuf v1.36.5), update kcp and sdk/client (and/or root) to match, consolidate/remove duplicate genproto entries, then run go mod tidy across affected modules.
🤖 Prompt for AI Agents
In go.mod around line 52 (google.golang.org/grpc v1.70.0) and across the repo
(kcp/go.mod, sdk/client/go.mod, root go.mod, cli and docs modules), gRPC,
protobuf and genproto versions are inconsistent; pick canonical versions (e.g.,
google.golang.org/grpc v1.70.0 and google.golang.org/protobuf v1.36.5), update
kcp/go.mod and sdk/client/go.mod (and/or root) to those versions,
consolidate/remove duplicate google.golang.org/genproto entries so all modules
reference the same genproto version, then run go mod tidy in each affected
module to regenerate go.sum and ensure builds are consistent.
|
@coderabbitai disable yourself |
|
@xrstf I'll disable automatic reviews for this PR. ✅ Actions performedReviews paused. |
Summary
v2.1.0 was from 2022 and had quite a few bugfixes and dependency bumps done to it, so it's worth updating.
I also removed the seemingly unused
cloud.google.com/goreplace directive and restructured the go.mod for better visibility.What Type of PR Is This?
/kind cleanup
Release Notes
Summary by CodeRabbit
New Features
Documentation
Chores