Skip to content

HYPERFLEET-1147 - feat: enable caller identity header in API helm values#60

Open
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1147-caller-identity
Open

HYPERFLEET-1147 - feat: enable caller identity header in API helm values#60
kuudori wants to merge 1 commit into
openshift-hyperfleet:mainfrom
kuudori:HYPERFLEET-1147-caller-identity

Conversation

@kuudori

@kuudori kuudori commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Configure identity_header in base API values so the API resolves caller identity from the X-HyperFleet-Identity HTTP header for audit attribution. Overridable via API_IDENTITY_HEADER env var.

Summary

  • HYPERFLEET-1147

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

@openshift-ci openshift-ci Bot requested review from jsell-rh and mliptak0 June 25, 2026 20:05
@openshift-ci

openshift-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ldornele for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
📝 Walkthrough

Summary by CodeRabbit

  • New Features
    • Added support for configuring the API identity HTTP header via an environment variable, with a default value when not set.
    • The identity header is now included in adapter status-reporting requests (for both cluster and node pool status) across base and E2E configurations.

Walkthrough

Adds config.server.identity_header to the Helm values template from API_IDENTITY_HEADER with a default of X-HyperFleet-Identity, and updates adapter post actions to send X-HyperFleet-Identity: adapter@adapter.local on cluster and nodepool status requests. CWE-345.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the caller-identity header Helm change and its API focus.
Description check ✅ Passed The description matches the change set and explains the identity header plus env override.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed No changed log statements include token/password/credential/secret; touched files only add YAML headers/config, with no CWE-532-style secret logging found.
No Hardcoded Secrets ✅ Passed No CWE-798/259/321 secret patterns: only X-HyperFleet-Identity and adapter@adapter.local literals appear; no keys, tokens, passwords, private keys, base64 blobs, or credential URLs.
No Weak Cryptography ✅ Passed PASS: Modified Helm/YAML files only add identity_header/header values; no crypto imports, SHA1/HMAC, ECB, custom crypto, or secret comparisons (CWE-327/328/208).
No Injection Vectors ✅ Passed No new CWE-89/78/79/502 sinks: touched files only add static YAML/header config; scan found no fmt.Sprintf, exec.Command, template.HTML, or yaml.Unmarshal.
No Privileged Containers ✅ Passed No privileged container settings were added in the touched Helm/YAML; no privileged/host*/allowPrivilegeEscalation/runAsUser:0/SYS_ADMIN found (CWE-266).
No Pii Or Sensitive Data In Logs ✅ Passed No slog/logr/zap/fmt.Print statements were added; new email-like strings are YAML header values, not logs (CWE-532 not implicated).

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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

Configure identity_header in base API values so the API resolves caller
identity from the X-HyperFleet-Identity HTTP header for audit attribution.
Overridable via API_IDENTITY_HEADER env var.
@kuudori kuudori force-pushed the HYPERFLEET-1147-caller-identity branch from 4c52514 to 0ea5f21 Compare June 25, 2026 20:55

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml`:
- Around line 184-185: The adapter task config is hardcoding the identity header
name instead of using the same configurable source as the API. Update the header
entry in the adapter task configuration to be driven by the rendered value
derived from API_IDENTITY_HEADER, matching the identity header setting used by
base-api.yaml.gotmpl and consumed by hyperfleet-api at runtime. Keep the
identity value as-is, but make the header name overridable through the same
config path so status calls continue sending the header the API expects when the
env var is changed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: fadd9cc0-a428-4bea-ae3f-f0baf153d1a5

📥 Commits

Reviewing files that changed from the base of the PR and between 4c52514 and 0ea5f21.

📒 Files selected for processing (9)
  • helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml
  • helmfile/configs/base/adapters/adapter2/adapter-task-config.yaml
  • helmfile/configs/base/adapters/adapter3/adapter-task-config.yaml
  • helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
  • helmfile/configs/e2e/adapters/cl-job/adapter-task-config.yaml
  • helmfile/configs/e2e/adapters/cl-maestro/adapter-task-config.yaml
  • helmfile/configs/e2e/adapters/cl-namespace/adapter-task-config.yaml
  • helmfile/configs/e2e/adapters/np-configmap/adapter-task-config.yaml
  • helmfile/values/base-api.yaml.gotmpl
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift-hyperfleet/architecture (manual)
  • openshift-hyperfleet/hyperfleet-api (manual)
  • openshift-hyperfleet/hyperfleet-sentinel (manual)
  • openshift-hyperfleet/hyperfleet-adapter (manual)
  • openshift-hyperfleet/hyperfleet-broker (manual)
✅ Files skipped from review due to trivial changes (2)
  • helmfile/configs/e2e/adapters/cl-deployment/adapter-task-config.yaml
  • helmfile/values/base-api.yaml.gotmpl

Comment on lines +184 to +185
- name: "X-HyperFleet-Identity"
value: "adapter@adapter.local"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not hardcode the identity header name.

base-api.yaml.gotmpl makes config.server.identity_header configurable via API_IDENTITY_HEADER, and hyperfleet-api consumes that setting at runtime. Hardcoding X-HyperFleet-Identity here means any override will stop adapter status calls from sending the header the API actually reads, breaking audit attribution for these updates (CWE-345). Drive this header name from the same rendered value source as the API config.

As per path instructions, "All env vars are defined with ?= in the env files, so values can be overridden on the CLI (use this to verify API_IDENTITY_HEADER overrides behavior)."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@helmfile/configs/base/adapters/adapter1/adapter-task-config.yaml` around
lines 184 - 185, The adapter task config is hardcoding the identity header name
instead of using the same configurable source as the API. Update the header
entry in the adapter task configuration to be driven by the rendered value
derived from API_IDENTITY_HEADER, matching the identity header setting used by
base-api.yaml.gotmpl and consumed by hyperfleet-api at runtime. Keep the
identity value as-is, but make the header name overridable through the same
config path so status calls continue sending the header the API expects when the
env var is changed.

Sources: Path instructions, Linked repositories

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