feat: add --repo-server-address flag to skip port-forward in repo-server-api mode#407
Open
roulettedares wants to merge 4 commits into
Open
feat: add --repo-server-address flag to skip port-forward in repo-server-api mode#407roulettedares wants to merge 4 commits into
roulettedares wants to merge 4 commits into
Conversation
…ver-api mode When running argocd-diff-preview inside the same cluster as Argo CD (e.g. a self-hosted CI runner), the repo server is reachable directly via ClusterIP DNS. Port-forwarding through the Kubernetes API server is unnecessary in this case and requires pods/portforward RBAC that would otherwise not be needed. This commit wires up the existing but unused NewClientWithAddress() constructor by adding a --repo-server-address flag. When set: - repo-server-api mode connects directly to the provided host:port - EnsurePortForward() is a no-op (returns nil when k8sClient is nil) - No pods/portforward RBAC is required on the target cluster The flag is validated to only be used with --render-method=repo-server-api. TLS is enabled with InsecureSkipVerify=true (matching the port-forward path, since the repo server uses a self-signed certificate by default). Example: argocd-diff-preview --render-method repo-server-api --repo-server-address argocd-repo-server.argocd.svc.cluster.local:8081 ... Fixes: NewClientWithAddress() was defined but never called (pkg/reposerver/client.go)
- pkg/reposerver: verify NewClientWithAddress sets fields correctly, EnsurePortForward is a no-op when k8sClient is nil, and Cleanup does not panic on a direct-address client - cmd: verify ToConfig accepts --repo-server-address with repo-server-api and rejects it with server-api / cli render methods
- options.md: add flag and REPO_SERVER_ADDRESS env var to options table - rendering-methods.md: add in-cluster direct-connect section under repo-server-api - reusing-clusters/self-hosted-gh-runner.md: split RBAC into minimal (--repo-server-address) and broad options
…ss path - Remove multi-line doc comments from tests where function names are self-explanatory - Move pods/portforward explanation from YAML comment to prose in rendering-methods.md - Fix appofapps.go to honour --repo-server-address (was silently ignored for app-of-apps traversal) - Trim redundant EnsurePortForward docstring to a single line
Owner
|
@roulettedares Thank you for the contribution and you showing interest in the project - I really appreciate it! In general, I try to keep the tool as seamless as possible and avoid adding too many arguments if possible, since that can be confusing for new users. Would you be open to making this automatic instead? We already do service discovery in other places. For example, in Could we use the same pattern for repo server access, so users do not need to provide an extra argument in the common case? 🚀 Let me know what you think! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When running argocd-diff-preview inside the same cluster as Argo CD (e.g. a self-hosted CI runner), the repo server is reachable directly via ClusterIP DNS. Port-forwarding through the Kubernetes API server is unnecessary in this case and requires pods/portforward RBAC that would otherwise not be needed.
This commit wires up the existing but unused NewClientWithAddress() (defined in pkg/reposerver/client.go:77) constructor by adding a --repo-server-address flag. When set:
Useful when running inside the same cluster as Argo CD.
The flag is validated to only be used with --render-method=repo-server-api. TLS is enabled with InsecureSkipVerify=true (matching the port-forward path, since the repo server uses a self-signed certificate by default).
Example:
Fixes: NewClientWithAddress() was defined but never called (pkg/reposerver/client.go)