Add a "dropNetworking" function and unit tests to the runner package. #3582
Conversation
|
The following is the coverage report on the affected files.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg 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 |
|
Loving these small PRs!!!! 😍 |
This will be used for TEP-0025 [Hermekton](https://github.com/tektoncd/community/blob/master/teps/0025-hermekton.md) to drop network access for the process run by the runner. The unit tests here aren't great, but are better than nothing IMO. They can only run on Linux, and test that network access is not available. To do this, they try to make a network request. If the tests were to run in a sandboxed environment, we would miss this. They will be reinforced by e2e tests that properly verify network access exists without this call, and then doesn't exist with this call.
|
The following is the coverage report on the affected files.
|
|
This lgtm but i already approved it. @tektoncd/core-maintainers any objections to merging this? |
| Size: 4294967295, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
this function is beautiful 🤩
| "github.com/google/go-cmp/cmp" | ||
| ) | ||
|
|
||
| // This isn't a great unit test, but it's the best I can think of. |
There was a problem hiding this comment.
this is such a nitpick but i think if you start docstrings with anything other than the name of the function it's confusing for tools
| // It attempts to verify there is no network access by making a network | ||
| // request. If the test were to run in an offline environment, or an already | ||
| // sandboxed environment, the test could pass even if the dropNetworking | ||
| // function did nothing. |
There was a problem hiding this comment.
i guess the other thing you could do is assert on the exact values you set on the cmd object but that doesn't feel super useful
Changes
This will be used for TEP-0025 Hermekton
to drop network access for the process run by the runner.
The unit tests here aren't great, but are better than nothing IMO. They can only run on Linux, and test that
network access is not available. To do this, they try to make a network request. If the tests were to run
in a sandboxed environment, we would miss this.
They will be reinforced by e2e tests that properly verify network access exists without this call, and then doesn't
exist with this call.
/kind feature
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes