OCPBUGS-82974: Make IRI registry read-only via environment variable#5845
OCPBUGS-82974: Make IRI registry read-only via environment variable#5845openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@rwsu: This pull request references Jira Issue OCPBUGS-82974, which is invalid:
Comment 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. |
|
/jira refresh |
|
@rwsu: This pull request references Jira Issue OCPBUGS-82974, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds the environment variable REGISTRY_STORAGE_MAINTENANCE_READONLY={"enabled":true} to the IRI registry systemd unit ExecStart and updates the unit-rendering test to assert the rendered unit Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@rwsu: This pull request references Jira Issue OCPBUGS-82974, which is valid. 3 validation(s) were run on this bug
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. |
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/controller/internalreleaseimage/internalreleaseimage_helpers_test.go`:
- Line 43: The test currently only checks for presence of the key substring in
ignCfg.Systemd.Units[0].Contents which is too permissive; update the assertion
to verify the exact env assignment string (e.g. assert.Contains t,
*ignCfg.Systemd.Units[0].Contents, "REGISTRY_STORAGE_MAINTENANCE_READONLY=1")
and also add an explicit negative assertion to reject the incorrect variant
(e.g. assert.NotContains t, *ignCfg.Systemd.Units[0].Contents,
"REGISTRY_STORAGE_MAINTENANCE_READONLY_ENABLED") so the test fails if the broken
`_ENABLED` form is produced.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d2bd2814-5808-4ed5-96cd-025d55bc8962
📒 Files selected for processing (2)
pkg/controller/internalreleaseimage/internalreleaseimage_helpers_test.gopkg/controller/internalreleaseimage/templates/master/units/iri-registry.service.yaml
|
/test unit |
|
/verified by @rwsu |
|
@rwsu: 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. |
|
/lgtm |
|
Scheduling tests matching the |
|
Will also require to run a green iso-no-registry job @rwsu |
|
/hold as per @andfasano 's comments |
|
/test ? |
|
/test e2e-agent-compact-ipv4-iso-no-registry |
…v var
Without this change, clients pushing to the IRI registry receive HTTP 500
Internal Server Error. Write requests reach the :ro filesystem mount and
fail there, causing the registry to return 500. With
REGISTRY_STORAGE_MAINTENANCE_READONLY='enabled: true' set, push and delete
requests are rejected at the application layer with 405 Method Not Allowed
instead.
REGISTRY_STORAGE_MAINTENANCE_READONLY_ENABLED=true does not work -- the env
var parser produces map[string]interface{} but the registry code expects
map[interface{}]interface{} from gopkg.in/yaml.v2, causing a panic. Passing
the entire YAML map as the value at path depth 1
(REGISTRY_STORAGE_MAINTENANCE_READONLY='enabled: true') triggers
yaml.Unmarshal() and produces the correct type.
Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@rwsu: This pull request references Jira Issue OCPBUGS-82974, which is valid. 3 validation(s) were run on this bug
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. |
|
/test e2e-agent-compact-ipv4-iso-no-registry |
|
/test e2e-agent-compact-ipv4-iso-no-registry |
1 similar comment
|
/test e2e-agent-compact-ipv4-iso-no-registry |
|
/verified by @rwsu |
|
@rwsu: 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. |
|
/lgtm |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andfasano, pawanpinjarkar, rwsu 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 |
|
e2e-agent-compact-ipv4-iso-no-registry passed |
|
/test e2e-aws-ovn |
|
@rwsu: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@rwsu: Jira Issue Verification Checks: Jira Issue OCPBUGS-82974 Jira Issue OCPBUGS-82974 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. |
- What I did
Without this change, clients pushing to the IRI registry receive HTTP 500 Internal Server Error. Write requests reach the :ro filesystem mount and fail there, causing the registry to return 500. With REGISTRY_STORAGE_MAINTENANCE_READONLY='enabled: true' set, push and delete requests are rejected at the application layer with 405 Method Not Allowed instead.
REGISTRY_STORAGE_MAINTENANCE_READONLY_ENABLED=true does not work -- the env var parser produces map[string]interface{} but the registry code expects map[interface{}]interface{} from gopkg.in/yaml.v2, causing a panic. Passing the entire YAML map as the value at path depth 1
(REGISTRY_STORAGE_MAINTENANCE_READONLY='enabled: true') triggers yaml.Unmarshal() and produces the correct type.
Fixes: OCPBUGS-82974
- How to verify it
When iri-registry.service is running, push any image to it and you should receive status 405 error:
Error: writing blob: initiating layer upload to /v2/test/image/blobs/uploads/ in localhost:22625: StatusCode: 405, "Method not allowed\n"- Description for the changelog
Enforce IRI registry read-only mode at the application layer to return 405 instead of 500 on write attempts.
Summary by CodeRabbit
New Features
Tests