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

run: add support for inline --network in RUN statements #4566

Merged
merged 2 commits into from
Mar 14, 2023

Conversation

flouthoc
Copy link
Collaborator

@flouthoc flouthoc commented Feb 8, 2023

Buildah should allow clients to support inline --network in RUN stmts so users
can create isolate or expose a particular build containers.

FROM alpine
RUN --network=host wget google.com
RUN --network=none wget google.com

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

run: add support for inline --network in RUN stmt

Closes: #4230

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved label Feb 8, 2023
@flouthoc
Copy link
Collaborator Author

flouthoc commented Feb 8, 2023

/hold wait for openshift/imagebuilder#247 and a rebase so this PR does not points to a fork.

@TomSweeneyRedHat
Copy link
Member

LGTM once the imagebuilder PR is merged and you vendor from that.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 17, 2023
@flouthoc
Copy link
Collaborator Author

flouthoc commented Mar 9, 2023

Will rebase this after: openshift/imagebuilder#247

@rhatdan
Copy link
Member

rhatdan commented Mar 9, 2023

It's merged.

@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 9, 2023
@flouthoc
Copy link
Collaborator Author

flouthoc commented Mar 9, 2023

@rhatdan @TomSweeneyRedHat Rebased. @containers/buildah-maintainers PTAL

run_linux.go Outdated Show resolved Hide resolved
run_linux.go Outdated Show resolved Hide resolved
We need commit openshift/imagebuilder@598142f

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
Copy link
Collaborator Author

@rhatdan @nalind PTAL again.

Note: failed tests are flakes, restarted them.

tests/bud.bats Outdated Show resolved Hide resolved
tests/bud.bats Show resolved Hide resolved
@flouthoc flouthoc force-pushed the run-network branch 2 times, most recently from dc9d40c to c70284f Compare March 13, 2023 18:10
tests/bud.bats Outdated
@test "build with inline RUN --network=host" {
run_buildah build $WITH_POLICY_JSON -t source -f $BUDFILES/inline-network/Dockerfile1
expect_output --substring "Connecting to google.com"
expect_output --substring "index.html"
Copy link
Member

@nalind nalind Mar 13, 2023

Choose a reason for hiding this comment

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

Perhaps this should be running readlink /proc/self/ns/net both during inside and after the build and comparing the two.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True a better test will be readlink /proc/self/ns/net

@flouthoc
Copy link
Collaborator Author

@nalind @rhatdan @vrothberg @giuseppe @containers/buildah-maintainers PTAL

Buildah should allow clients to support inline --network in RUN stmts so users
can create isolate or expose a particular build containers.

```Dockerfile
FROM alpine
RUN --network=host wget google.com
RUN --network=none wget google.com
```

Closes: containers#4230

Signed-off-by: Aditya R <arajan@redhat.com>
@nalind
Copy link
Member

nalind commented Mar 14, 2023

LGTM

@flouthoc
Copy link
Collaborator Author

@rhatdan @containers/buildah-maintainers PTAL

@rhatdan
Copy link
Member

rhatdan commented Mar 14, 2023

/lgtm

@rhatdan
Copy link
Member

rhatdan commented Mar 14, 2023

/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit b5060e2 into containers:main Mar 14, 2023
flouthoc added a commit to flouthoc/common that referenced this pull request Mar 15, 2023
Document RUN --network added in containers/buildah#4566

[NO NEW TESTS NEEDED]
[CI:DOCS]

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/common that referenced this pull request Mar 15, 2023
Document RUN --network added in containers/buildah#4566

[NO NEW TESTS NEEDED]
[CI:DOCS]

Signed-off-by: Aditya R <arajan@redhat.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Containerfile with RUN --network none ... Does not work.
5 participants