Bug 1940488: move entitlement related secrets back to mounts.conf

[gabemontero]

So the use of buildah transient mounts for the host entitlement files in #228 was ultimately undone by the fact that core libs for buildah/podman always automount system-fips to /run/secrets and based on how the various mounts are ordered when the build container is getting built, that system-fips mount overwrote our host entitlement mounts to /run/secrets.

According to @nalind, changing the mount point destination to /run/secrets/system-fips is not an option across the board for the non-build, podman related scenarios.

Now, when we re-introduce the use of mounts.conf for our entitlement mounts, the core libs order things correctly for us and both system-fips and our entitlement mounts show up.

I have verified this PR in the UPI test cluster @wewang58 has set up for us, using an openshift/builder image from this PR that I built.

Also, @nalind and I have talked at length, and confirmed that we can use both mounts.conf and transient mounts for the optional trust CAs feature/fix that @adambkaplan has recently provided. There is no way for us to add that path "optionally" using the mounts.conf approach. But as the optional trust CAs land in /etc/pki/ca-trust directly, there are no overlapping mount point concerns with what gets put into mounts.conf by openshift/builder or the defaults that buildah/podman/etc. inject under the covers on us.

There is an alternative approach of changing the buildah code specifically (not the core libs) to change its ordering so that the system-fips happens before our entitlement mounts, thus both are preserved. Conceptually mimicking how mounts.conf is being handled.

But as the list of customer cases piles up for this regression of function we introduced when we fixed the CVE https://bugzilla.redhat.com/show_bug.cgi?id=1916897, the path length there is much longer, as we have to change buildah, get agreement / merge from that community, ideally but not necessary get a new version tag cut, and then vendor back into openshift/builder. Some of these customer cases are on the brink of escalation.

So I am pursing a faster, openshift/builder only path, with this PR.

That said, I think making that buildah only change for the ordering would be good long term, and if it eventually happened, when we vendor that change in, we could move back to transient mounts only and remove mounts.conf again if we chose.

/assign @nalind
as he has been my main collaborator on this ... looking for an official lgtm from him, unless it also comes from
/assign @bparees
who with @adambkaplan out we need for getting the approve label

[openshift-ci-robot]

@gabemontero: This pull request references Bugzilla bug 1940488, which is invalid:

• expected the bug to be in one of the following states: NEW, ASSIGNED, ON_DEV, POST, POST, but it is ON_QA instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

WIP: Bug 1940488: move entitlement related secrets back to mounts.conf

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/test-infra repository.

[gabemontero]

/bugzilla refresh

[openshift-ci-robot]

@gabemontero: This pull request references Bugzilla bug 1940488, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.8.0) matches configured target release for branch (4.8.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @wewang58

Details

In response to this:

/bugzilla refresh

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/test-infra repository.

[gabemontero]

unrelated e2e failures

/retest

[openshift-ci-robot]

@gabemontero: This pull request references Bugzilla bug 1940488, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.8.0) matches configured target release for branch (4.8.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @wewang58

Details

In response to this:

Bug 1940488: move entitlement related secrets back to mounts.conf

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/test-infra repository.

[gabemontero]

so @nalind if we are running on RHCOS, with no entitlements, whatever flavor of error log we get from buildah if these files are missing are the only possible cause for concern perhaps

I'm going to try and bring up a IPI / rhcos cluster today, use a builder image from this PR, and get a sense of what if anything that looks like. Based on the results, we'll see if that steers us back to just changing the ordering in buildah

but of course let me know what you think

@wewang58 adding a regression test case with any build on a normal IPI / RHCOS / clusterbot cluster of this PR to see what the logs look like makes sense for you ... if the e2e's pass, the builds work, but we are just worried about scary but non-fatal messages in the build log

[nalind]

IIRC it's not a fatal error message, but it's not something we have an API for suppressing.

[gabemontero]

OK I'm launching a RHCOS cluster now. I'll get an example of what the message looks like and we can decide if it is a show stopper for this approach.

[nalind]

I expect something along the lines of time="2021-03-30T21:08:24Z" level=warning msg="Path \"/run/secrets/redhat.repo\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping" for each RUN instruction.

[gabemontero]

hmmmm ... not terrible, if that is all ... though perhaps if there are a lot of RUN's and they pile up

my RHCOS cluster is no up, hope to confirm soon

[gabemontero]

yep confirmed @nalind ... the following occurs with each RUN:

time="2021-03-31T17:16:09Z" level=warning msg="Path \"/run/secrets/etc-pki-entitlement\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping"
time="2021-03-31T17:16:09Z" level=warning msg="Path \"/run/secrets/redhat.repo\" from \"/etc/containers/mounts.conf\" doesn't exist, skipping"

wdyt @bparees .... too noisy?

[bparees]

given the apparent urgency to get a resolution here, i'd say we can merge this for now if we can clean it up after.

[gabemontero]

ok thanks @bparees

since it will still be a day or two before we can pick to the 4.7 z stream, and I have test clusters up, I'll spend some time today developing locally the "change buildah ordering" alternative mentioned in the description that @nalind and I have discussed, just to explicitly asses its viability while I still have access to UPI test envs and the like

in the interim we can see if the escalation warnings come to fruition, etc.

[nalind]

@rhatdan is taking a run at it in containers/buildah#3117

[gabemontero]

awesome thanks @nalind

If my POC works with the buildah change I'll post a WIP/do not merge PR up for us to compare

[gabemontero]

/assign @bparees
/assign @nalind

[nalind]

LGTM

[openshift-ci-robot]

@gabemontero: This pull request references Bugzilla bug 1940488, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target release (4.8.0) matches configured target release for branch (4.8.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @wewang58

Details

In response to this:

Bug 1940488: move entitlement related secrets back to mounts.conf

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/test-infra repository.

[bparees]

/approve

[bparees]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, gabemontero

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [bparees]

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

[openshift-ci-robot]

@gabemontero: All pull requests linked via external trackers have merged:

• openshift/builder#228
• openshift/builder#238

Bugzilla bug 1940488 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1940488: move entitlement related secrets back to mounts.conf

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/test-infra repository.