Skip to content

Allow passing registry credentials via RunnerOptions - #4816

Closed
adi-coderr wants to merge 1 commit into
kptdev:mainfrom
adi-coderr:fix/runneroptions-registry-credentials
Closed

adi-coderr wants to merge 1 commit into
kptdev:mainfrom
adi-coderr:fix/runneroptions-registry-credentials

Conversation

@adi-coderr

Copy link
Copy Markdown
Contributor

This Fixes: #4813

Description

Previously, RunnerOptions only allowed passing custom TLS certificates (ExtraTlsCerts), while registry credentials were hardcoded to regclient.WithDockerCreds().

In custom runners and multi-tenant environments (such as Porch's pod evaluator), container execution supports private registries via Kubernetes Secrets (imagePullSecrets), but tag resolution and listing on the kpt side would fail with 401 Unauthorized / repository not found because regclient had no access to the corresponding registry credentials.

This change introduces RegistryCredentials in RunnerOptions so callers can supply credentials for private registries when resolving tags and listing images.

Key Changes

  1. Added RegistryCredentials to RunnerOptions (pkg/lib/runneroptions):

    • Supports structured Hosts (RegistryHost) with usernames, passwords, tokens, and TLS settings.
    • Supports raw DockerConfigJSON ([]byte) to consume Kubernetes Secrets (e.g. secret.Data[".dockerconfigjson"] or .dockercfg) directly without writing files to disk.
    • Supports DockerConfigFile for custom file paths.
    • Supports DisableDefaultDockerCreds to enable strict multi-tenant isolation by disabling host/node ambient credentials.
  2. Security Considerations:

    • Secret Masking: Implemented fmt.Stringer, fmt.GoStringer, and json.Marshaler for both RegistryCredentials and RegistryHost to ensure credentials (Password, Token, ClientKey, DockerConfigJSON) are always redacted ([REDACTED]) in logs, error traces, and JSON serialization.
    • In-Memory Parsing: Parses Docker config JSON directly in memory without writing temporary files to /tmp.
    • Process Isolation: Avoids modifying process-wide environment variables (DOCKER_AUTH_CONFIG), preventing race conditions and cross-tenant credential leakage in concurrent environments like Porch.
  3. regclient Integration (pkg/fn/runtime/runner.go):

    • Passes custom hosts to regclient.WithConfigHost(...) and optional config files to regclient.WithDockerCredsFile(...).
    • Custom credentials take precedence over ambient docker credentials for matching registry hosts.

Verification & Testing

  • Unit Tests (pkg/lib/runneroptions/credentials_test.go):
    • Validates parsing of standard config.json (auths), base64 credentials, tokens, credential helpers, and legacy .dockercfg.
    • Verifies credential redaction with String(), GoString(), and MarshalJSON().
  • Lister & Runner Authentication Tests (pkg/fn/runtime):
    • TestRegClientLister_Authentication: Verifies 401 Unauthorized without credentials vs 200 OK with credentials against private registry endpoints.
    • TestNewRunner_RegistryCredentials: Verifies runner initialization with Hosts, DockerConfigJSON, and DockerConfigFile, including live semver tag resolution (^1.0 -> v1.1.0 / v1.2.0) against private registries.
  • Docker E2E Tests:
    • make test-fn-eval T=".*": All 60+ containerized KRM function evaluation tests pass.
    • make test-fn-render T="image-pull-policy.*": All image pull policy tests pass.

@adi-coderr
adi-coderr requested review from a team and a balanced review from Copilot October 18, 2026 15:00
@netlify

netlify Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for kptdocs ready!

Name Link
🔨 Latest commit 2645bee
🔍 Latest deploy log https://app.netlify.com/projects/kptdocs/deploys/6ad539f7d535565a4ee652b1
😎 Deploy Preview https://deploy-preview-4816--kptdocs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Credential-store handling, Docker Hub helper lookup, and empty-config parsing can still break valid registry configurations.

3 open issues
What changed in this PR

Adds caller-supplied registry credentials to kpt’s library runner, enabling private-registry tag resolution for custom runners such as Porch.

Changes:

  • Adds structured, JSON, and file-based credentials with secret redaction.
  • Integrates credentials into regclient with optional ambient-credential isolation.
  • Adds parsing, redaction, and authenticated tag-resolution tests.
File Description
pkg/​lib/​runneroptions/​runneroptions.go Exposes registry credentials in runner options.
pkg/​lib/​runneroptions/​credentials.go Implements credential conversion, parsing, and redaction.
pkg/​lib/​runneroptions/​credentials_test.go Tests credential parsing and masking.
pkg/​fn/​runtime/​runner.go Configures regclient with supplied credentials.
pkg/​fn/​runtime/​runner_test.go Tests runner credential handling and tag resolution.
pkg/​fn/​runtime/​lister_test.go Tests authenticated registry tag listing.

Review effort: Balanced

Comment thread pkg/lib/runneroptions/credentials.go
Comment thread pkg/lib/runneroptions/credentials.go Outdated
Comment thread pkg/lib/runneroptions/credentials.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Tenant-supplied helper directives can access evaluator-local credentials despite disabling default Docker credentials.

1 open issue
3 resolved since last review

Review effort: Balanced

Comment thread pkg/lib/runneroptions/credentials.go
@adi-coderr
adi-coderr force-pushed the fix/runneroptions-registry-credentials branch from fc7fa63 to 3a90b84 Compare October 18, 2026 20:26
@adi-coderr
adi-coderr requested a balanced review from Copilot October 18, 2026 20:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Store-only Docker configurations still silently discard authentication when ambient credentials are disabled.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread pkg/lib/runneroptions/credentials.go Outdated
@adi-coderr
adi-coderr force-pushed the fix/runneroptions-registry-credentials branch from 3a90b84 to bff8fba Compare October 18, 2026 21:20
Signed-off-by: adi-coderr <personalworkadi@gmail.com>
@adi-coderr
adi-coderr force-pushed the fix/runneroptions-registry-credentials branch from bff8fba to 2645bee Compare October 18, 2026 21:28
@adi-coderr
adi-coderr requested a balanced review from Copilot October 18, 2026 21:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Credential-helper execution lacks cancellation bounds, and several tests depend on local credential-store contents.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment on lines +397 to +399
cmd := exec.Command(helper, "list")
cmd.Stdin = strings.NewReader("")
if out, err := cmd.Output(); err == nil {
Comment on lines +229 to +231
hosts, err = parseDockerConfigJSON([]byte(`{"credsStore":"pass"}`), true)
require.NoError(t, err)
assert.Nil(t, hosts)
@sonarqubecloud

Copy link
Copy Markdown

@adi-coderr adi-coderr closed this Oct 18, 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.

Eval/Render must allow passing of image registry credentials on the library level

2 participants