Skip to content

Conversation

@gnunn1
Copy link
Contributor

@gnunn1 gnunn1 commented Dec 3, 2025

What does this PR do / why we need it:

Fixes a couple of minor issues with the Agent instructions for cert-manager, specifically:

  • Suffixes the secret with -principal in the certificate since later instructions rely on that
  • Fixes the hardcoded managed-cluster for the commonName, this should be the cluster-name

Which issue(s) this PR fixes:

None

How to test changes / Special notes to the reviewer:

Follow instructions for using cert-manager for PKI

Checklist

  • Documentation update is required by this PR (and has been updated) OR no documentation update is required.

Summary by CodeRabbit

  • Documentation
    • Updated PKI certificate and agent setup docs to use new cluster-specific naming for secrets and certificate common names.
    • Sample CLI commands and output examples updated to show agent-specific secret names and file outputs.
    • Server URL examples now use https and include an agent query parameter; guidance added to ensure each agent URL is unique to avoid cache collisions.
    • Labeling examples switched to use an agent-name label and kubectl-based commands.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 3, 2025

Walkthrough

Documentation updates for PKI certificate and agent secret naming: example Certificate manifests, secret creation/labeling commands, agent YAML paths, and server URL examples were updated to use <cluster-name>-principal, <cluster-name>-agent, agent-specific server query parameter, HTTPS prefix, and agent-name labels; notes about unique server URLs and labeling were added.

Changes

Cohort / File(s) Summary
PKI & Agent docs
docs/configuration/agent/pki-certificates.md
Updated example Certificate fields: spec.secretName -> <cluster-name>-principal, spec.commonName -> <cluster-name>. Renamed CLI/output secret references to <cluster-name>-agent. Adjusted agent YAML file name/path examples, kubectl-neat / yq steps, and agent certificate commonName. Added/updated instructions to use HTTPS and include an agentName query parameter on the server URL to avoid cache collisions. Modified secret labeling examples to include agent-name label and switch documented tooling reference from oc to kubectl. Clarified labeling and matching agent notes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single documentation file but multiple related example changes and CLI snippets to verify.
  • Pay attention to: certificate manifest snippets, all CLI examples (secret creation/extraction), server URL examples (HTTPS + query param), and labeling commands.

Suggested reviewers

  • jgwest
  • chetan-rns
  • mikeshng
  • jannfis

Poem

🐇 I hopped through lines and renamed with care,

Secrets snugged up, labels all aware,
A query to tell each agent apart,
HTTPS smiles and tidy certs at heart,
🥕 Small hops, big clarity — a rabbit's cheer!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: minor corrections to cert-manager certificate documentation for the Agent, which aligns with the actual changes made to the PKI certificates documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gnunn1
Copy link
Contributor Author

gnunn1 commented Dec 3, 2025

Please don't merge this yet, I need to look at this one more time with a clear head.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be12f46 and 9b8f852.

📒 Files selected for processing (1)
  • docs/configuration/agent/pki-certificates.md (5 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/configuration/agent/pki-certificates.md

237-237: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ 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). (2)
  • GitHub Check: Build and push image
  • GitHub Check: Analyze (go)


```
kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=argocd-agent-resource-proxy.argocd.svc.cluster.local --from-file=config=./config
kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=https://server=argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<managed-agent> --from-file=config=./config
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix the malformed server URL in the secret creation command.

The server URL syntax is incorrect: https://server=argocd-agent-resource-proxy... duplicates server= and is not valid. Additionally, the placeholder should use <cluster-name> for consistency with the naming convention used elsewhere in the document.

Apply this diff to fix the syntax and placeholder:

-kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=https://server=argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<managed-agent> --from-file=config=./config
+kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=https://argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<cluster-name> --from-file=config=./config
📝 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.

Suggested change
kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=https://server=argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<managed-agent> --from-file=config=./config
kubectl create secret generic <cluster-name>-cluster -n argocd --from-literal=name=<cluster-name> --from-literal=server=https://argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<cluster-name> --from-file=config=./config
🤖 Prompt for AI Agents
In docs/configuration/agent/pki-certificates.md around line 227, the kubectl
secret creation command contains a malformed server URL ("https://server=...")
and uses an inconsistent placeholder for the agent name; replace the duplicated
"server=" with a proper host URL and use the <cluster-name> placeholder for the
agentName query parameter so the server literal becomes
"https://argocd-agent-resource-proxy.argocd.svc.cluster.local:9090?agentName=<cluster-name>"
while keeping --from-literal=name=<cluster-name> and
--from-file=config=./config.

Then label the secret as a cluster secret:
Then label the secret as a cluster secret and include the label to identify the matching agent:

```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add language specification to the fenced code block.

Markdown linting standards require a language identifier for code fences to enable proper syntax highlighting.

Apply this diff:

-```
+```bash
 kubectl label secret <cluster-name>-cluster argocd.argoproj.io/secret-type=cluster
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

237-237: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
In docs/configuration/agent/pki-certificates.md around line 237, the fenced code
block lacks a language identifier; update the code fence to include "bash" so it
becomes ```bash before the kubectl command to satisfy markdown linting and
enable proper syntax highlighting.

Comment on lines +238 to +239
kubectl label secret <cluster-name>-cluster argocd.argoproj.io/secret-type=cluster
kubectl label argocd-agent.argoproj-labs.io/agent-name=<cluster-name>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix incomplete kubectl label command.

The second labeling command is missing the secret resource and name specification. Both labels should be applied to the same secret resource.

Apply this diff to fix the command:

-kubectl label secret <cluster-name>-cluster argocd.argoproj.io/secret-type=cluster
-kubectl label argocd-agent.argoproj-labs.io/agent-name=<cluster-name>
+kubectl label secret <cluster-name>-cluster argocd.argoproj.io/secret-type=cluster
+kubectl label secret <cluster-name>-cluster argocd-agent.argoproj-labs.io/agent-name=<cluster-name>
🤖 Prompt for AI Agents
In docs/configuration/agent/pki-certificates.md around lines 238-239 the second
kubectl label command is missing the resource type and name; update it so both
labels are applied to the same secret resource. Replace the second line with a
command that targets the secret (e.g., kubectl label secret
<cluster-name>-cluster argocd-agent.argoproj-labs.io/agent-name=<cluster-name>)
or consolidate into a single kubectl label secret <cluster-name>-cluster
argocd.argoproj.io/secret-type=cluster
argocd-agent.argoproj-labs.io/agent-name=<cluster-name> so both labels are
applied to the <cluster-name>-cluster secret.


```
kubectl get secret managed-cluster-agent -o yaml -n argocd | oc neat > <cluster-name>-agent.yaml
kubectl get secret <cluster-name>-agent -o yaml -n argocd | oc neat > <cluster-name>-agent.yaml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace oc neat with kubectl-neat for consistency.

The documentation references kubectl-neat as the tool name (line 279-280), but the commands use oc neat. For consistency and clarity, use the standard kubectl command with the kubectl-neat plugin syntax.

Apply this diff:

-kubectl get secret <cluster-name>-agent -o yaml -n argocd | oc neat > <cluster-name>-agent.yaml
+kubectl get secret <cluster-name>-agent -o yaml -n argocd | kubectl neat > <cluster-name>-agent.yaml
-kubectl get secret argocd-agent-ca -o yaml -n argocd | yq 'del(.data.["tls.key"])' -y | oc neat > argocd-agent-ca.yaml
+kubectl get secret argocd-agent-ca -o yaml -n argocd | yq 'del(.data.["tls.key"])' -y | kubectl neat > argocd-agent-ca.yaml

Also applies to: 290-290

🤖 Prompt for AI Agents
docs/configuration/agent/pki-certificates.md lines 276 and 290: the docs
currently pipe output to "oc neat"; replace those with the kubectl plugin syntax
by piping to "kubectl neat" (i.e., use kubectl get ... | kubectl neat > ...) so
the command uses the standard kubectl client and the kubectl-neat plugin
consistently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant