OCPBUGS-78190: Pass image policy env setting to assisted#10379
OCPBUGS-78190: Pass image policy env setting to assisted#10379openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@bfournie: This pull request references Jira Issue OCPBUGS-78190, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
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:
WalkthroughRead OPENSHIFT_INSTALL_EXPERIMENTAL_DISABLE_IMAGE_POLICY during ignition/template generation and set DisableImagePolicy in template data so the assisted-service systemd unit and the Podman-run container receive the environment variable and optional ExecStart argument when enabled. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as "Installer/Build"
participant Generator as "Ignition Template Generator"
participant Systemd as "systemd unit (host)"
participant Podman as "Podman (container runtime)"
participant Service as "Assisted Service (container)"
rect rgba(0,128,255,0.5)
Builder->>Generator: invoke template generation
end
Generator->>Generator: read env OPENSHIFT_INSTALL_EXPERIMENTAL_DISABLE_IMAGE_POLICY\nset DisableImagePolicy flag
Generator->>Systemd: render assisted-service.service.template\n(include PassEnvironment and ExecStart -e when flag true)
Systemd->>Systemd: unit file contains env propagation and ExecStart args
Systemd->>Podman: systemd starts Podman with ExecStart
Podman->>Service: container inherits OPENSHIFT_INSTALL_EXPERIMENTAL_DISABLE_IMAGE_POLICY
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@bfournie: This pull request references Jira Issue OCPBUGS-78190, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
data/data/agent/systemd/units/assisted-service.service.template
Outdated
Show resolved
Hide resolved
bfournie
force-pushed
the
agent-sigstore-env
branch
from
79b1c86 to
b5f8e2b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/asset/agent/image/ignition.go`:
- Around line 426-431: Replace the current ad-hoc check that sets
disableImagePolicy by any non-empty
OPENSHIFT_INSTALL_EXPERIMENTAL_DISABLE_IMAGE_POLICY value with a shared helper
(e.g., parseCIOnlyBoolEnv or IsCIEnvBoolSet) that: 1) strictly parses the env
var using strconv.ParseBool so only true/false semantics are accepted, and 2)
enforces the CI/test-only boundary (e.g., require a CI marker like "CI" or
another agreed test-only env to be true) before returning true; then call that
helper instead of the existing code in both ignition paths that set
disableImagePolicy (replace current logic around disableImagePolicy and the
logrus.Warn) so the bypass is only enabled when the flag is explicitly true and
allowed by the CI guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2566c63-94d6-49a4-8034-62003985dda6
📒 Files selected for processing (3)
data/data/agent/systemd/units/assisted-service.service.templatepkg/asset/agent/image/ignition.gopkg/asset/agent/image/unconfigured_ignition.go
🚧 Files skipped from review as they are similar to previous changes (1)
- data/data/agent/systemd/units/assisted-service.service.template
bfournie
force-pushed
the
agent-sigstore-env
branch
from
b5f8e2b to
7e12492
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/asset/agent/image/ignition.go`:
- Around line 427-434: Extract the duplicated environment parsing into a shared
helper (e.g., isExperimentalImagePolicyDisabled) that performs os.LookupEnv +
strconv.ParseBool and logs the warning exactly as in the current ignition.go
logic, then replace the inline parsing in ignition.go (disableImagePolicy),
unconfigured_ignition.go, and cvoignore.go to call this helper so all code paths
use the same strict boolean parsing and identical warning message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74ffa1d9-fc45-400f-8481-2d33a61c706a
📒 Files selected for processing (3)
data/data/agent/systemd/units/assisted-service.service.templatepkg/asset/agent/image/ignition.gopkg/asset/agent/image/unconfigured_ignition.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/asset/agent/image/unconfigured_ignition.go
bfournie
force-pushed
the
agent-sigstore-env
branch
from
7e12492 to
eb10664
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/asset/agent/image/ignition_test.go`:
- Line 122: TestIgnition_getTemplateData assumes
OPENSHIFT_INSTALL_EXPERIMENTAL_DISABLE_IMAGE_POLICY is unset; save the current
value of that env var, unset it (or set to empty) before calling
getTemplateData(...) to build templateData, then restore the original value at
the end of the test; update the TestIgnition_getTemplateData test to perform
this save/unset/restore around the call that produces templateData so the
assertion on templateData.DisableImagePolicy is isolated from CI environment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2a3a4f7c-143f-4ae2-b268-98f73b8a1e2b
📒 Files selected for processing (4)
data/data/agent/systemd/units/assisted-service.service.templatepkg/asset/agent/image/ignition.gopkg/asset/agent/image/ignition_test.gopkg/asset/agent/image/unconfigured_ignition.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/asset/agent/image/ignition.go
- data/data/agent/systemd/units/assisted-service.service.template
- pkg/asset/agent/image/unconfigured_ignition.go
bfournie
force-pushed
the
agent-sigstore-env
branch
from
eb10664 to
046ea13
Compare
|
/approve |
Nightly builds are currently failing to install for agent jobs because the images are not signed. Pass in the environment setting from CI that will disable the check. This will only be true in CI.
bfournie
force-pushed
the
agent-sigstore-env
branch
from
046ea13 to
37ecfae
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andfasano The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Verified with local testing using nightly images by setting /verified by @bfournie |
openshift-ci-robot
added
the
verified
|
@bfournie: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-merge-bot
bot
merged commit a6ba91c
into
openshift:main
|
@bfournie: Jira Issue Verification Checks: Jira Issue OCPBUGS-78190 Jira Issue OCPBUGS-78190 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
4 participants
Nightly builds are currently failing to install for agent jobs because the images are not signed. Pass in the environment setting from CI that will disable the check. This will only be true in CI.
Summary by CodeRabbit