Skip to content

remove --registry- flags#34

Merged
bennyz merged 3 commits into
centos-automotive-suite:mainfrom
bennyz:remove-sensitive
Jan 21, 2026
Merged

remove --registry- flags#34
bennyz merged 3 commits into
centos-automotive-suite:mainfrom
bennyz:remove-sensitive

Conversation

@bennyz

@bennyz bennyz commented Jan 21, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation

    • Updated registry authentication docs, examples, header notes, and troubleshooting to show credentials provided via REGISTRY_USERNAME and REGISTRY_PASSWORD environment variables and to reflect env-var-first behavior.
  • Refactor

    • Registry credential handling now sources credentials from environment variables first, falls back to Docker/Podman auth, and no longer supports supplying registry username/password via CLI flags.

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

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
@coderabbitai

coderabbitai Bot commented Jan 21, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Registry authentication moved from CLI flags to environment variables (REGISTRY_USERNAME / REGISTRY_PASSWORD); extractRegistryCredentials now returns (URL, username, password) and logic falls back to Docker/Podman auth when env vars are absent.

Changes

Cohort / File(s) Summary
Documentation
cmd/caib/README.md
Removed --registry-username / --registry-password flags from help text and examples; updated examples and troubleshooting to document REGISTRY_USERNAME / REGISTRY_PASSWORD env-var precedence and Docker/Podman fallback.
Credential handling
cmd/caib/main.go
Changed extractRegistryCredentials(primaryRef, secondaryRef string) signature to return (string, string, string) (registry URL, username, password). Removed global registryUsername / registryPassword vars and CLI flags; added validateRegistryCredentials and updated call sites to set RegistryCredentials only when URL, username, and password are provided.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (caib)
    participant Fn as extractRegistryCredentials()
    participant Env as Environment (REGISTRY_USERNAME / REGISTRY_PASSWORD)
    participant Docker as Docker/Podman auth
    participant Reg as Registry
    CLI->>Fn: request registry creds for push/export
    Fn->>Env: check REGISTRY_USERNAME / REGISTRY_PASSWORD
    alt env creds present
        Env-->>Fn: return (URL, username, password)
    else
        Fn->>Docker: read ~/.docker/config.json or podman auth
        Docker-->>Fn: return (URL, token/creds) or none
    end
    Fn-->>CLI: return (URL, username, password)
    CLI->>Reg: push/export using returned credentials (if provided)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through flags and found a trail,
Env vars now carry the secret veil.
From Docker burrow or ENV-lit glen,
Credentials come — I push again. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: removing registry-related CLI flags from the codebase, which aligns with the substantial changes across README and main.go.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)
cmd/caib/main.go (1)

389-418: Avoid sending empty registry credentials in build requests.

When REGISTRY_USERNAME/REGISTRY_PASSWORD aren’t set, extractRegistryCredentials returns empty values but these blocks still set RegistryCredentials, which can override fallback and cause auth failures. Gate on both fields (as runDisk already does), or error when only one is set.

🛠️ Suggested fix
-	if effectiveRegistryURL != "" {
+	if effectiveRegistryURL != "" && registryUsername != "" && registryPassword != "" {
 		req.RegistryCredentials = &buildapitypes.RegistryCredentials{
 			Enabled:     true,
 			AuthType:    "username-password",
 			RegistryURL: effectiveRegistryURL,
 			Username:    registryUsername,
 			Password:    registryPassword,
 		}
 	}
-	if effectiveRegistryURL != "" {
+	if effectiveRegistryURL != "" && registryUsername != "" && registryPassword != "" {
 		req.RegistryCredentials = &buildapitypes.RegistryCredentials{
 			Enabled:     true,
 			AuthType:    "username-password",
 			RegistryURL: effectiveRegistryURL,
 			Username:    registryUsername,
 			Password:    registryPassword,
 		}
 	}

Also applies to: 754-781

🤖 Fix all issues with AI agents
In `@cmd/caib/main.go`:
- Around line 121-126: The registry-detection logic incorrectly only checks for
a dot; update the condition in the function that derives registry from the image
reference (the block using parts := strings.SplitN(ref, "/", 2) and returning
parts[0]) to treat the first path component as a registry when it contains a '.'
OR ':' OR equals "localhost" (case-sensitive per spec). Replace the current
strings.Contains(parts[0], ".") check with a predicate that checks
strings.Contains(parts[0], ".") || strings.Contains(parts[0], ":") || parts[0]
== "localhost", and apply the same change to the other two call sites that use
this same extraction logic so localhost and host:port registries are correctly
recognized. Ensure return values remain username, password as before.

Comment thread cmd/caib/main.go
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cmd/caib/main.go (1)

389-418: Avoid sending empty registry credentials in build/build-dev requests

With env vars now being the sole source, RegistryCredentials can be sent with empty username/password (Enabled=true), which risks failed auth or overriding defaults. Gate on non-empty creds (align with runDisk).

🔧 Proposed fix
-	if effectiveRegistryURL != "" {
+	if effectiveRegistryURL != "" && registryUsername != "" && registryPassword != "" {
 		req.RegistryCredentials = &buildapitypes.RegistryCredentials{
 			Enabled:     true,
 			AuthType:    "username-password",
 			RegistryURL: effectiveRegistryURL,
 			Username:    registryUsername,
 			Password:    registryPassword,
 		}
 	}
-	if effectiveRegistryURL != "" {
+	if effectiveRegistryURL != "" && registryUsername != "" && registryPassword != "" {
 		req.RegistryCredentials = &buildapitypes.RegistryCredentials{
 			Enabled:     true,
 			AuthType:    "username-password",
 			RegistryURL: effectiveRegistryURL,
 			Username:    registryUsername,
 			Password:    registryPassword,
 		}
 	}

Also applies to: 754-781

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
@bennyz bennyz merged commit 325d6b8 into centos-automotive-suite:main Jan 21, 2026
4 checks passed
@bennyz bennyz deleted the remove-sensitive branch January 21, 2026 18:31
@coderabbitai coderabbitai Bot mentioned this pull request Feb 8, 2026
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