docs: refine credential system documentation#732
Conversation
✅ Deploy Preview for open-component-model ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ff961af to
1072a07
Compare
9cbcffb to
e340cdd
Compare
|
I like how you start the concept, and explain why we have a central credential system. Glossary / terminology, all clean and helpful. Then you start mixing the different diataxis types: concept (explanation), reference and how-to in one doc. I propose to move the identity-matching table and config discovery paths to a reference doc, and extract "Putting It All Together" into a How-to. Especially the huge table in Identity Matching section and the many tables with fields distracts a lot. Maybe we rename the concept to "Credential System" as the list with OCM... gets longer in the concepts section :-) |
e340cdd to
7034c14
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a unified credential system doc set (concept, reference, how‑to, tutorials), removes an old tutorial, updates internal links to the new credential docs, and adds "XDG" to the project wordlist. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/docs/concepts/credential-system.md`:
- Line 143: Replace the fragile relative relref tag `{{< relref
"configure-credentials-for-controllers.md" >}}` with an absolute docs path used
across the PR (e.g. `{{< relref
"docs/concepts/configure-credentials-for-controllers.md" >}}`) so the link is
stable across sections; update the markdown line that currently reads
`[Credentials for OCM Controllers]({{< relref
"configure-credentials-for-controllers.md" >}})` to use the absolute relref
string instead.
In `@content/docs/how-to/configure-multiple-credentials.md`:
- Around line 82-107: The docs example uses the deprecated field name
`pathprefix` for OCIRepository identities; update the example and explanatory
text to use the v2 `path` field with glob semantics instead of `pathprefix`, and
adjust the explanatory lines (e.g., "Uses `pathprefix`..." and the
"Identity/Matches" text) to describe `path` glob matching and the precedence
rule (more specific `path` patterns checked first); ensure the example consumer
identity still references `type: OCIRepository`, `hostname: ghcr.io` and the
pattern (e.g., a glob-style `path`) so the snippet and the surrounding prose are
consistent with v2.
- Line 206: The relref link for "OCM Credentials" is missing a leading slash;
update the link target in the "[OCM Credentials]" line so the relref uses a
leading '/' (e.g., change {{< relref "docs/concepts/credential-system.md" >}} to
include the leading slash) to match other docs and ensure correct resolution by
the relref helper.
In `@content/docs/tutorials/credential-resolution.md`:
- Around line 263-289: The docs incorrectly claim that OCM "treats `oci` like
`https` and explicitly sets port 443"; update Example D and the surrounding
explanation to state that the hostpath identity builder does not auto-fill a
port for the `oci` scheme (only `https`→443 and `http`→80 have defaults). In the
paragraph referencing `oci://` behavior (Example D), remove the sentence about
treating `oci` like `https` and replace it with: "Unlike `https` or `http`, the
`oci` scheme has no built-in default port; if a port is not included in the URL
the port attribute remains empty." Ensure the table and the example
YAML/description remain consistent with this corrected behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8286af7f-8f27-4718-8b68-83126084ad39
📒 Files selected for processing (12)
.github/config/wordlist.txtcontent/docs/concepts/credential-system.mdcontent/docs/getting-started/deploy-helm-chart.mdcontent/docs/getting-started/setup-controller-environment.mdcontent/docs/how-to/configure-multiple-credentials.mdcontent/docs/how-to/v1-credential-compatibility.mdcontent/docs/reference/credential-configuration.mdcontent/docs/tutorials/configure-credentials-for-controllers.mdcontent/docs/tutorials/credential-resolution.mdcontent/docs/tutorials/creds-in-ocmconfig.mdcontent/docs/tutorials/deploy-helm-chart-bootstrap.mdcontent/docs/tutorials/signing-and-verification.md
💤 Files with no reviewable changes (1)
- content/docs/tutorials/creds-in-ocmconfig.md
7034c14 to
7a10fbb
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
content/docs/concepts/credential-system.md (1)
143-143:⚠️ Potential issue | 🟡 MinorUse an absolute relref here for consistency and stability.
This link still uses a relative target while the rest of this PR standardizes absolute docs relrefs.
Suggested fix
-- [Credentials for OCM Controllers]({{< relref "configure-credentials-for-controllers.md" >}}) — how to provide credentials in Kubernetes environments +- [Credentials for OCM Controllers]({{< relref "/docs/tutorials/configure-credentials-for-controllers.md" >}}) — how to provide credentials in Kubernetes environments🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/docs/concepts/credential-system.md` at line 143, The link "[Credentials for OCM Controllers]({{< relref "configure-credentials-for-controllers.md" >}})" uses a relative relref; update it to the project's standardized absolute relref form (replace the relative target "configure-credentials-for-controllers.md" with the absolute path used across the PR) so the entry becomes an absolute relref consistent with other docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@content/docs/concepts/credential-system.md`:
- Line 143: The link "[Credentials for OCM Controllers]({{< relref
"configure-credentials-for-controllers.md" >}})" uses a relative relref; update
it to the project's standardized absolute relref form (replace the relative
target "configure-credentials-for-controllers.md" with the absolute path used
across the PR) so the entry becomes an absolute relref consistent with other
docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 826eaf27-6e14-44c0-9123-66b203d2e40a
📒 Files selected for processing (12)
.github/config/wordlist.txtcontent/docs/concepts/credential-system.mdcontent/docs/getting-started/deploy-helm-chart.mdcontent/docs/getting-started/setup-controller-environment.mdcontent/docs/how-to/configure-multiple-credentials.mdcontent/docs/how-to/v1-credential-compatibility.mdcontent/docs/reference/credential-configuration.mdcontent/docs/tutorials/configure-credentials-for-controllers.mdcontent/docs/tutorials/credential-resolution.mdcontent/docs/tutorials/creds-in-ocmconfig.mdcontent/docs/tutorials/deploy-helm-chart-bootstrap.mdcontent/docs/tutorials/signing-and-verification.md
💤 Files with no reviewable changes (1)
- content/docs/tutorials/creds-in-ocmconfig.md
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/config/wordlist.txt
- content/docs/how-to/configure-multiple-credentials.md
- content/docs/reference/credential-configuration.md
- content/docs/tutorials/credential-resolution.md
- content/docs/how-to/v1-credential-compatibility.md
- content/docs/tutorials/configure-credentials-for-controllers.md
7a10fbb to
2d1c119
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
content/docs/tutorials/configure-credentials-for-controllers.md (1)
66-83:⚠️ Potential issue | 🟠 MajorUpdate the sample config to v2 schema to match the new docs.
After Line 63 points readers to the v2 credential system, the example at Lines 69–83 still uses v1-style fields. That mismatch will confuse copy/paste usage.
Proposed doc fix
type: generic.config.ocm.software/v1 configurations: - type: credentials.config.ocm.software consumers: - - identity: - type: OCIRegistry + - identities: + - type: OCIRepository scheme: https hostname: ghcr.io - pathprefix: <your-namespace> + path: <your-namespace> credentials: - - type: Credentials + - type: Credentials/v1 properties: username: <your-username> password: <your-password/token>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/docs/tutorials/configure-credentials-for-controllers.md` around lines 66 - 83, The example YAML uses v1 fields but the docs reference the v2 credential system; update the sample to the v2 schema by changing the resource version from generic.config.ocm.software/v1 to /v2 and by replacing the v1-style credential fields shown (the credentials.config.ocm.software block, consumers.identity keys: type, scheme, hostname, pathprefix, and credentials.properties username/password) with their v2 equivalents per the v2 credential schema; ensure the sample reflects v2 field names and structure exactly (e.g., new top-level version and the v2 layout for credential references/secrets) so the snippet matches the rest of the docs.
♻️ Duplicate comments (1)
content/docs/tutorials/credential-resolution.md (1)
263-266:⚠️ Potential issue | 🟠 MajorRemove the incorrect
oci://default-port claim.Line 263/265 currently says OCM treats
ocilikehttpsand sets port 443 explicitly. That contradicts the rest of the section and leads to a wrong mental model.Proposed correction
-Some OCI tools use `oci://` URLs instead of `https://`. When OCM builds a lookup identity from an `oci://` URL, it treats `oci` like `https` and explicitly sets **port 443**. However, the identity matcher only has built-in port defaults for `https` → `443` and `http` → `80` — the `oci` scheme has **no default port**. - -In practice this works because OCM sets port `443` explicitly on both sides for `oci://` URLs, so they match via literal comparison. +Some OCI tools use `oci://` URLs instead of `https://`. Unlike `https` or `http`, the `oci` scheme has **no built-in default port** in matching. If a port is not present in the URL, the lookup identity keeps an empty port value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/docs/tutorials/credential-resolution.md` around lines 263 - 266, The paragraph incorrectly claims OCM treats the `oci` scheme like `https` and explicitly sets port 443; update the text in the credential-resolution doc to remove that claim and instead state that the identity matcher only has built-in default ports for `https` → `443` and `http` → `80`, and that `oci` has no default port so matching relies on explicit ports when present. Edit the sentences referencing `oci://` and port 443 (the lines describing OCM building a lookup identity from an `oci://` URL) to clarify that OCM does not implicitly map `oci` to `https`/443 and ensure the example explanation reflects that matching works only when both sides explicitly include the same port.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/docs/tutorials/credential-resolution.md`:
- Around line 200-203: The table row for the Request `ghcr.io/my-org/repo`
(Example B2) contradicts the documented "first-match-wins" rule: update that row
so the Result is the Consumer A credential (`org-user`) and change the Why
column to reflect "A matches first (first-match-wins)" instead of saying B is
more specific; ensure the sample config labels ("Consumer A", "Consumer B") and
the Result/Why text align with the first-match-wins rule.
---
Outside diff comments:
In `@content/docs/tutorials/configure-credentials-for-controllers.md`:
- Around line 66-83: The example YAML uses v1 fields but the docs reference the
v2 credential system; update the sample to the v2 schema by changing the
resource version from generic.config.ocm.software/v1 to /v2 and by replacing the
v1-style credential fields shown (the credentials.config.ocm.software block,
consumers.identity keys: type, scheme, hostname, pathprefix, and
credentials.properties username/password) with their v2 equivalents per the v2
credential schema; ensure the sample reflects v2 field names and structure
exactly (e.g., new top-level version and the v2 layout for credential
references/secrets) so the snippet matches the rest of the docs.
---
Duplicate comments:
In `@content/docs/tutorials/credential-resolution.md`:
- Around line 263-266: The paragraph incorrectly claims OCM treats the `oci`
scheme like `https` and explicitly sets port 443; update the text in the
credential-resolution doc to remove that claim and instead state that the
identity matcher only has built-in default ports for `https` → `443` and `http`
→ `80`, and that `oci` has no default port so matching relies on explicit ports
when present. Edit the sentences referencing `oci://` and port 443 (the lines
describing OCM building a lookup identity from an `oci://` URL) to clarify that
OCM does not implicitly map `oci` to `https`/443 and ensure the example
explanation reflects that matching works only when both sides explicitly include
the same port.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e799d0c-e0fc-43ae-ab34-7d5aab196815
📒 Files selected for processing (12)
.github/config/wordlist.txtcontent/docs/concepts/credential-system.mdcontent/docs/getting-started/deploy-helm-chart.mdcontent/docs/getting-started/setup-controller-environment.mdcontent/docs/how-to/configure-multiple-credentials.mdcontent/docs/how-to/v1-credential-compatibility.mdcontent/docs/reference/credential-configuration.mdcontent/docs/tutorials/configure-credentials-for-controllers.mdcontent/docs/tutorials/credential-resolution.mdcontent/docs/tutorials/creds-in-ocmconfig.mdcontent/docs/tutorials/deploy-helm-chart-bootstrap.mdcontent/docs/tutorials/signing-and-verification.md
💤 Files with no reviewable changes (1)
- content/docs/tutorials/creds-in-ocmconfig.md
🚧 Files skipped from review as they are similar to previous changes (3)
- content/docs/how-to/configure-multiple-credentials.md
- content/docs/how-to/v1-credential-compatibility.md
- .github/config/wordlist.txt
892d4e1 to
9660114
Compare
<!-- markdownlint-disable MD041 --> Replaces the old v1-era credential tutorial (creds-in-ocmconfig.md) with a complete set of credential documentation structured using the Diataxis framework. All examples use the OCM v2 configuration format (identities plural, OCIRepository type, path instead of pathprefix, Credentials/v1). **New pages:** - **Concept** — concepts/credential-system.md: explains consumers, identities, credential types, and repository fallback - **Tutorial** — tutorials/credential-resolution.md: hands-on walkthrough of identity matching with 6 worked examples (hostname match, glob path, two-level wildcard, scheme/port normalization, oci scheme, no-match scenarios) - **How-to** — how-to/configure-multiple-credentials.md: step-by-step guide for multi-registry credentials with Docker config fallback - **How-to** — how-to/v1-credential-compatibility.md: migration guide from OCM v1 to v2 credential format (identity to identities, OCIRegistry to OCIRepository, pathprefix to path, Credentials to Credentials/v1) **Deleted pages:** - tutorials/creds-in-ocmconfig.md: superseded by the new pages above **Updated links:** - signing-and-verification.md, deploy-helm-chart-bootstrap.md, configure-credentials-for-controllers.md, setup-controller-environment.md, deploy-helm-chart.md — updated cross-references to point to the new credential pages <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> Fixes: open-component-model/ocm-project#888 Signed-off-by: Piotr Janik <piotr.janik@sap.com>
9660114 to
f0eb59e
Compare
Co-authored-by: Gerald Morrison <67469729+morri-son@users.noreply.github.com> Signed-off-by: Piotr Janik <piotrjanik@nautilia.pl>
<!-- markdownlint-disable MD041 --> #### What this PR does / why we need it Replaces the old v1-era credential tutorial (creds-in-ocmconfig.md) with a complete set of credential documentation structured using the Diataxis framework. All examples use the OCM v2 configuration format (identities plural, OCIRepository type, path instead of pathprefix, Credentials/v1). **New pages:** - **Concept** — concepts/credential-system.md: explains consumers, identities, credential types, and repository fallback - **Tutorial** — tutorials/credential-resolution.md: hands-on walkthrough of identity matching with 6 worked examples (hostname match, glob path, two-level wildcard, scheme/port normalization, oci scheme, no-match scenarios) - **How-to** — how-to/configure-multiple-credentials.md: step-by-step guide for multi-registry credentials with Docker config fallback - **How-to** — how-to/v1-credential-compatibility.md: migration guide from OCM v1 to v2 credential format (identity to identities, OCIRegistry to OCIRepository, pathprefix to path, Credentials to Credentials/v1) - **Reference** — reference/credential-configuration.md: field-level schema, identity matching rules, and config discovery paths **Deleted pages:** - tutorials/creds-in-ocmconfig.md: superseded by the new pages above **Updated links:** - signing-and-verification.md, deploy-helm-chart-bootstrap.md, configure-credentials-for-controllers.md, setup-controller-environment.md, deploy-helm-chart.md — updated cross-references to point to the new credential pages #### Which issue(s) this PR fixes <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> Fixes: open-component-model/ocm-project#888 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added comprehensive credential system concept docs, a credential-configuration reference, and a credential-resolution tutorial explaining identity matching and resolution order * New how-to guide for configuring credentials across multiple OCI registries and a migration guide for v1→v2 credential configs * Enhanced signing & verification workflow docs, updated credential cross‑references, and removed an obsolete credential tutorial * **Chores** * Expanded allowed wordlist with the token "XDG" <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Piotr Janik <piotr.janik@sap.com> Co-authored-by: Piotr Janik <piotr@piotrs-macbook-pro.local> 8daf5aa
What this PR does / why we need it
Replaces the old v1-era credential tutorial (creds-in-ocmconfig.md) with a complete set of credential documentation structured using the Diataxis framework. All examples use the OCM v2 configuration format (identities plural, OCIRepository type, path instead of pathprefix, Credentials/v1).
New pages:
Deleted pages:
Updated links:
Which issue(s) this PR fixes
Fixes: open-component-model/ocm-project#888
Summary by CodeRabbit
Documentation
Chores