Skip to content

feat: add PPM authenticated repos via Identity Federation#112

Draft
ian-flores wants to merge 3 commits intomainfrom
ppm-auth-repos
Draft

feat: add PPM authenticated repos via Identity Federation#112
ian-flores wants to merge 3 commits intomainfrom
ppm-auth-repos

Conversation

@ian-flores
Copy link
Collaborator

Description

Enable Connect and Workbench to authenticate against PPM using Kubernetes Identity Federation (RFC 8693 token exchange). When authenticatedRepos: true is set on a product spec, the operator automatically injects init container + sidecar pods that exchange K8s projected service account tokens for PPM API tokens, writing netrc/curlrc files for Python and R package installation.

Issue

  • PTDC-224: Configure SSO (OIDC) for Package Manager web UI
  • PTDC-223: Configure Connect to automatically use PPM authenticated repos
  • PTDC-222: Configure Workbench to automatically use PPM authenticated repos

Code Flow

Authentication Flow

K8s Projected SA Token → Init Container → PPM /__api__/token (RFC 8693) → PPM API Token
                                                                                │
                                                                        Written to netrc + curlrc
                                                                                │
                         Connect/Workbench ← reads netrc/curlrc ←───────────────┘
                         (R: CURL_HOME, Python: NETRC env var)

Sidecar refreshes the token on a timer (default 50min for 60min token lifetime).

Key Components

  1. PPM OIDC Config Types (package_manager_config.go): New PackageManagerOIDCConfig and PackageManagerIdentityFederationConfig structs. Extended GenerateGcfg() to emit named [IdentityFederation "connect"] sections.

  2. Token Exchange Helpers (ppm_auth.go): Shared functions for init container, sidecar container, volumes, volume mounts, and env vars. Shell script in ConfigMap does the token exchange via curl+jq on alpine:3.

  3. Product Integration (connect.go, workbench.go): When AuthenticatedRepos=true, adds projected SA token volume, shared emptyDir, script ConfigMap volume, init container, sidecar container, NETRC + CURL_HOME env vars.

  4. Site Controller (site_controller.go): Creates {site}-ppm-auth-script ConfigMap when any product has authenticated repos enabled. Auto-configures Identity Federation entries on PPM based on product flags.

  5. OIDC Client Secret Mount (packagemanager_types.go, package_manager.go): Mounts the OIDC client secret at /etc/rstudio-pm/oidc-client-secret via SecretProviderClass (AWS) or K8s Secret volume (K8s).

Category of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have run just test and all tests pass
  • I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about

Enable Connect and Workbench to authenticate against PPM using
Kubernetes Identity Federation (RFC 8693 token exchange). Adds
OIDC and Identity Federation config types for PPM, shared token
exchange init container and sidecar helpers, and opt-in
AuthenticatedRepos flag on Connect/Workbench specs.
All 8 findings addressed. Build passes and tests pass. Here's the summary:
Changes:
- Install curl and jq in token exchange script (`apk add --no-cache`) so alpine:3 default image works
- Add null/empty token validation after jq extraction to fail fast instead of writing "null" as password
- Add `OIDCAudience` field to `SiteSpec` so OIDC audience is configurable (defaults to `sts.amazonaws.com` for backwards compatibility)
- Revert `AutomountServiceAccountToken` to `ptr.To(false)` in Connect — projected volume works independently
- Add `PPMAuthImage` to `InternalConnectSpec` and `InternalWorkbenchSpec` and propagate from Site controllers
- Add gcfg injection validation for `IdentityFederation` Name (reject `"`, `]`, newlines)
- Add `cleanupPPMAuthConfigMap` to delete the ConfigMap when authenticated repos feature is disabled
- Add `SanitizePPMUrl` helper to strip existing scheme before prepending `https://`, preventing double-prefix
@claude
Copy link

claude bot commented Mar 2, 2026

Claude finished @ian-flores's task —— View job


Review: feat: add PPM authenticated repos via Identity Federation

  • Read review guidelines
  • Analyze PR diff and changed files
  • Review API types and CRD changes
  • Review controller logic
  • Review PPM config generation
  • Review tests
  • Submit review

Overall

Well-structured feature. The separation of concerns (shared helpers in ppm_auth.go, product integration in connect.go/workbench.go, site-level orchestration in site_controller*.go) follows existing patterns. The token exchange flow via init container + sidecar is a solid approach.

Below are findings organized by severity.


Critical: Audience mismatch between projected SA token and Identity Federation config

internal/controller/core/ppm_auth.go:170 — The projected SA token uses audience = ppmURL (e.g., https://ppm.example.com).

internal/controller/core/site_controller_package_manager.go:139-141 — The IdentityFederation entry uses site.Spec.OIDCAudience (defaulting to "sts.amazonaws.com").

These must match for the token exchange to succeed. If the PPM-side IdentityFederation.Audience is "sts.amazonaws.com" but the projected token's aud claim is "https://ppm.example.com", PPM will reject the token. Either:

  • Pass the OIDC audience through to PPMAuthVolumes() instead of using the PPM URL, or
  • Document explicitly why these are intentionally different values and explain how the PPM token endpoint resolves this.

Important: Shell script installs packages at runtime

internal/controller/core/ppm_auth.go:36apk add --no-cache curl jq runs on every container start.

  1. Air-gapped environments: This will always fail if the Alpine mirror is unreachable.
  2. Supply chain risk: Pulls unsigned packages at runtime on each pod start.
  3. Startup latency: Adds network dependency to pod initialization.

Consider using a Posit-owned image with curl and jq pre-installed as the default. The PPMAuthImage override field is already there, so this is mainly about a better default. At minimum, document this requirement.


Important: Sidecar exits on transient token refresh failure

internal/controller/core/ppm_auth.go:74-79 — The script uses set -e, so if exchange_token fails during a sidecar refresh cycle (e.g., PPM temporarily unavailable), the entire sidecar dies. The token will expire and auth will break with no recovery.

Consider:

if [ "${MODE}" = "sidecar" ]; then
    while true; do
        sleep "$REFRESH_INTERVAL"
        exchange_token || echo "WARNING: token refresh failed, will retry" >&2
    done
fi

Important: Connect sets AutomountServiceAccountToken: false

internal/controller/core/connect.go:653 — Connect explicitly sets AutomountServiceAccountToken: ptr.To(false). The PPM auth flow uses a projected SA token volume (not the default automounted one), so this should still work. However, if the projected volume projection depends on the SA token being available at the kubelet level, verify this works correctly in testing. Workbench sets it to true (line 815), so there's no issue there.


Important: PPM URL computed without https:// prefix in site controller

internal/controller/core/site_controller_connect.go:159 and site_controller_workbench.go:261PPMUrl is set to prefixDomain(...) which returns a bare domain like ppm.example.com (no scheme). This gets passed to SanitizePPMUrl() in the product reconcilers which prepends https://. This works but is fragile — the PPM URL meaning shifts between layers. Consider adding the scheme at the site controller level for clarity.


Suggestion: Remove committed .claude/tsc-cache/ files

.claude/tsc-cache/3a39c930-cf12-4be4-a2f2-eae1467b53c6/ — These are local roborev tool artifacts (the edited-files.log contains macOS temp paths). They should be removed from this PR and .claude/tsc-cache/ added to .gitignore.


Suggestion: Unused constant

internal/controller/core/ppm_auth.go:20ppmAuthCurlrcPath is defined but never referenced in Go code (the shell script uses its own CURLRC_PATH variable). Remove to avoid confusion.


Suggestion: Default audience assumption

internal/controller/core/site_controller_package_manager.go:141 — Defaulting audience to "sts.amazonaws.com" is AWS-specific. The field comment mentions AKS, but the default is EKS-only. Consider requiring this field when AuthenticatedRepos is enabled instead of silently defaulting to an AWS value.


API Checklist

  • Kubebuilder annotations present (+optional)
  • New fields have sensible defaults (booleans default to false, strings to empty)
  • No validation rules for PPMUrl or OIDCIssuerURL — consider adding URL format validation
  • Non-breaking (additive only)

Controller Checklist

  • Reconciliation is idempotent (ConfigMap uses CreateOrUpdate, cleanup handles NotFound)
  • Error handling reports status
  • Config flows from Site → Product correctly
  • Unit tests for ppm_auth.go helpers and package_manager_config.go
  • No integration/controller tests for the end-to-end auth flow (ConfigMap creation, product reconciliation with AuthenticatedRepos=true)

Helm Chart

  • CRDs are up to date (both config/crd and dist/chart match)

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review posted via comment due to tool permissions. See the Claude comment on this PR for detailed feedback.

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