OCPBUGS-77141: Fix registriesd "unknown userid" failure for arbitrary UIDs#1357
OCPBUGS-77141: Fix registriesd "unknown userid" failure for arbitrary UIDs#1357openshift-merge-bot[bot] merged 2 commits intoopenshift:mainfrom
Conversation
|
@lunarwhite: This pull request references Jira Issue OCPBUGS-77141, 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 |
|
@lunarwhite: This pull request references Jira Issue OCPBUGS-77141, 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: 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. |
|
/cc @aguidirh |
|
@lunarwhite: This pull request references Jira Issue OCPBUGS-77141. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state. 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. |
|
@lunarwhite: This pull request references Jira Issue OCPBUGS-77141, 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. |
| // os.UserHomeDir() reads $HOME env var first, avoiding passwd lookups. | ||
| homeDir, err := os.UserHomeDir() | ||
| if err == nil { | ||
| return registriesDirPathWithHomeDir(homeDir), nil |
There was a problem hiding this comment.
If the user has a home directory but does not have ~/.config/containers/registries.d yet, we silently pivot to use systemRegistriesDirPath. I think it makes more sense to only use systemRegistriesDirPath when we can't find $HOME.
There was a problem hiding this comment.
Resolved after talking with @aguidirh , feel free to resolve this
| // os.UserHomeDir() reads $HOME env var first, avoiding passwd lookups. | ||
| homeDir, err := os.UserHomeDir() | ||
| if err == nil { | ||
| return registriesDirPathWithHomeDir(homeDir), nil |
There was a problem hiding this comment.
Resolved after talking with @aguidirh , feel free to resolve this
aguidirh
left a comment
There was a problem hiding this comment.
Thanks for fixing this bug. This is an critical one.
|
/label acknowledge-critical-fixes-only |
|
/cherry-pick release-4.21 |
|
@aguidirh: once the present PR merges, I will cherry-pick it on top of 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 kubernetes-sigs/prow repository. |
|
/verified by @nidangavali |
|
@nidangavali: 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adolfo-ab, aguidirh, lunarwhite, nidangavali 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 |
|
Thanks team for all your quick review! Could I get a |
|
@lunarwhite: Jira Issue Verification Checks: Jira Issue OCPBUGS-77141 Jira Issue OCPBUGS-77141 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. |
|
@aguidirh: new pull request created: #1358 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 kubernetes-sigs/prow repository. |
|
@lunarwhite: 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. |
Description
Github / Jira issue: https://issues.redhat.com/browse/OCPBUGS-77141
Background
The
registriesdpackage was introduced in v4.19 and later enabled by default in v4.21 to support sigstore signature attachments for mirrored container images. The module attempts to discover the user's home directory to locate registries.d configuration files using Go'suser.Current()function:oc-mirror/internal/pkg/registriesd/registriesd.go
Lines 52 to 57 in b27c4b9
We've met an edge case where
oc-mirrorrunning inside a container with arbitrary user IDs (i.e. OpenShift CI). The failure occurs during the sigstore signature preparation phase with the following error:Ref: https://github.com/openshift/release/blob/93e8fa82775a0ab6a81e85acccbd2e32e332360e/ci-operator/step-registry/cert-manager/install/catalog/cert-manager-install-catalog-commands.sh#L103-L128
Root Cause
user.Current()requires an/etc/passwdentry for the current UID. But in containerized environments, processes could run with dynamically assigned UIDs (e.g.1000890000or1003650000) that don't exist in/etc/passwd. And, this function would immediately return an error instead of falling back to environment variables or system defaults.Proposed Fix
Replace
user.Current()withos.UserHomeDir()inGetDefaultRegistrydConfigPath().This matches the pattern used in
executor.gofor cache directory setup:oc-mirror/internal/pkg/cli/executor.go
Line 223 in b27c4b9
Type of change
How Has This Been Tested?
without this patch: jobs failed at
oc-mirrorcommands (staring from Feb 4th).with this patch:
oc-mirrorcommands succeeded and jobs passed