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.

Sign up for GitHub

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

Jump to bottom

ValidatingWebhookConfig should fail on error #434

Closed
cmwylie19 opened this issue Dec 8, 2023 · 0 comments · Fixed by #436
Closed

ValidatingWebhookConfig should fail on error #434

cmwylie19 opened this issue Dec 8, 2023 · 0 comments · Fixed by #436
Assignees
@cmwylie19
Labels
enhancement New feature or request

Comments

@cmwylie19
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

Depending on validating webhook configuration, they can fail open when they are unreachable. Because Pepr is doing a security function, we want to fail on error. Ensure failurePolicy: Fail is set in ValidatingWebhookConfiguration

Describe the solution you'd like

  • Given an error evaluating a match condition
  • When the webhook is never called
  • Then reject the request

Describe alternatives you've considered

(optional) A clear and concise description of any alternative solutions or features you've considered.

Additional context

https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-matchconditions
@cmwylie19 cmwylie19 added the enhancement New feature or request label Dec 8, 2023
@cmwylie19 cmwylie19 self-assigned this Dec 12, 2023
@cmwylie19 cmwylie19 mentioned this issue Dec 12, 2023
Merged
5 tasks
cmwylie19 added a commit that referenced this issue Dec 13, 2023
@cmwylie19
chore: connect failurePolicy to package.json onError (#436)
1a229d1 
## Description

@bburky brought up a potential security thread related to the webhook
configurations's `failurePolicy`. Currently, the failurePolicy is always
ignore and there is no way to change it, this prove be danger to a
ValidatingAdmission Webook or even a Mutating that is supposed to remove
capability and securityContexts because it can cause the webhoook to
_ignore the object and pass it into the cluster_.

During `npx pepr init` there is a prompt about:

```bash
? How do you want Pepr to handle errors encountered during K8s operations? › - Use arrow-keys. Return to submit.
❯   Ignore - Pepr will continue processing and generate an entry in the Pepr Controller log.
    Log an audit event
    Reject the operation
```

Based on this response, the package.json is populated with `onError`
section:

```json
 "pepr": {
    "name": "pepr-test-module",
    "uuid": "static-test",
    "onError": "reject", // <----- HERE
    "alwaysIgnore": {
      "namespaces": [],
      "labels": []
    },
    "includedFiles": []
  },
```

If `reject` is present on the onError section then the failurePolicy
generated during a `npx pepr build` will be `Fail`.

## Related Issue

Fixes #434 
<!-- or -->
Relates to #

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/pepr/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed

---------

Signed-off-by: Case Wylie <cmwylie19@defenseunicorns.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cmwylie19 cmwylie19

Labels
enhancement New feature or request
Projects
Pepr Project Board
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: connect failurePolicy to package.json onError defenseunicorns/pepr
1 participant
@cmwylie19