Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: fix e2e test for privileged builds #10873

Merged
merged 1 commit into from
Aug 3, 2023

Conversation

milas
Copy link
Contributor

@milas milas commented Aug 3, 2023

What I did
We cannot guarantee the exact value of CapEff across environments, and this test has started failing some places, e.g. Docker Desktop, and now GitHub Actions (likely due to a kernel upgrade on the runners or similar).

By setting privileged: true on the build, we're asking for the security.insecure entitlement on the build. A safe assumption is that will include CAP_SYS_ADMIN, which won't be present otherwise, so mask the CapEff value and check for that.

It's worth noting that realistically, the build won't even be able to complete without the correct entitlement, since the Dockerfile uses RUN --security=insecure, so this is really an additional sanity check.

(not mandatory) A picture of a cute animal, if possible in relation to what you did
a cat escaping from a kennel

We cannot guarantee the exact value of `CapEff` across
environments, and this test has started failing some places,
e.g. Docker Desktop, and now GitHub Actions (likely due to
a kernel upgrade on the runners or similar).

By setting `privileged: true` on the build, we're asking for
the `security.insecure` entitlement on the build. A safe
assumption is that will include `CAP_SYS_ADMIN`, which won't
be present otherwise, so mask the `CapEff` value and check
for that.

It's worth noting that realistically, the build won't even
be able to complete without the correct entitlement, since the
`Dockerfile` uses `RUN --security=insecure`, so this is really
an additional sanity check.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas requested a review from a team August 3, 2023 13:57
@milas milas self-assigned this Aug 3, 2023
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team August 3, 2023 13:57
@milas
Copy link
Contributor Author

milas commented Aug 3, 2023

h/t @neersighted for reminding me how capabilities work and suggesting this approach like a month ago

Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

👍

@milas milas merged commit f94cb49 into docker:v2 Aug 3, 2023
22 of 23 checks passed
@milas milas deleted the fix-e2e-build-privileged branch August 3, 2023 16:23
kylos101 pushed a commit to gitpod-io/compose that referenced this pull request Aug 8, 2023
We cannot guarantee the exact value of `CapEff` across
environments, and this test has started failing some places,
e.g. Docker Desktop, and now GitHub Actions (likely due to
a kernel upgrade on the runners or similar).

By setting `privileged: true` on the build, we're asking for
the `security.insecure` entitlement on the build. A safe
assumption is that will include `CAP_SYS_ADMIN`, which won't
be present otherwise, so mask the `CapEff` value and check
for that.

It's worth noting that realistically, the build won't even
be able to complete without the correct entitlement, since the
`Dockerfile` uses `RUN --security=insecure`, so this is really
an additional sanity check.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
kylos101 pushed a commit to gitpod-io/compose that referenced this pull request Aug 8, 2023
We cannot guarantee the exact value of `CapEff` across
environments, and this test has started failing some places,
e.g. Docker Desktop, and now GitHub Actions (likely due to
a kernel upgrade on the runners or similar).

By setting `privileged: true` on the build, we're asking for
the `security.insecure` entitlement on the build. A safe
assumption is that will include `CAP_SYS_ADMIN`, which won't
be present otherwise, so mask the `CapEff` value and check
for that.

It's worth noting that realistically, the build won't even
be able to complete without the correct entitlement, since the
`Dockerfile` uses `RUN --security=insecure`, so this is really
an additional sanity check.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants