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

feat: add policy exemptions #165

Merged
merged 69 commits into from
Mar 7, 2024
Merged

feat: add policy exemptions #165

merged 69 commits into from
Mar 7, 2024

Conversation

TristanHoladay
Copy link
Contributor

@TristanHoladay TristanHoladay commented Feb 9, 2024

Description

This PR adds a way for users to register policy exemptions at runtime using the uds-core operator.

Related Issue

Relates to #130

Type of change

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

Checklist before merging

@TristanHoladay
Copy link
Contributor Author

TristanHoladay commented Feb 13, 2024

Ran several options with setting exemptions in the store.

  1. synchronously only using setItem() -- not too surprisingly doesn't work when writing to the same policy key multiple times (neuvector for example does this). Because of how the store works, if not given enough time only the last write makes it to the store and then most pods needing exemptions for that policy are denied.

  2. asynchronously with setItemAndWait() -- solves the concurrent write problem but is slow and exacerbates the issue of pods starting up before exemptions have made it into the store. In neuvector's case, this means the enforcer pod tries to run and is denied one or more times before successfully deploying.

  • trial 1: "msg":"Time to complete exemption write: 118758.07304999232"
  • trial 2: "msg":"Time to complete exemption write: 118881.12059801817"}
  1. synchronously with local aggregation first -- solves the concurrent write issue by first creating a local map and aggregating all matchers from the Exemption into lists attached to their relevant policy. Then a single write is made for each policy to the store without any waiting for the previous call to finish. This drastically reduces the time for the exemptions to show up in the store. There still seems to be cases of pods getting denied while waiting. However I noticed that they seemed to take less attempts and the neuvector deployment finished faster in this case.
  • trial 1: "msg":"Time to complete exemption write: 2.490468978881836"}
  • trial 2: "msg":"Time to complete exemption write: 0.44616997241973877"}
  • trial 3: "msg":"Time to complete exemption write: 0.9177009463310242"}
  • trial 4: "msg":"Time to complete exemption write: 0.4052410125732422"}

Note: these write times are not truly accurate for how long it took for the store to be updated with all exemptions. These numbers reflect how long it took to create the local map and then send each update to the store.

@TristanHoladay TristanHoladay marked this pull request as ready for review February 13, 2024 23:12
@TristanHoladay TristanHoladay requested a review from a team as a code owner February 13, 2024 23:12
Copy link
Member

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

Won't be able to review for a couple days due to in-person things, but wanted to flag to make sure I review this one as it's going to touch some of the same areas as the identity work.

Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

Think this is in a great spot overall - couple small comments here.

package.json Outdated Show resolved Hide resolved
src/pepr/operator/crd/generated/exemption-v1alpha1.ts Outdated Show resolved Hide resolved
src/pepr/operator/crd/generated/exemption-v1alpha1.ts Outdated Show resolved Hide resolved
src/pepr/policies/security.ts Outdated Show resolved Hide resolved
src/pepr/operator/index.ts Outdated Show resolved Hide resolved
.yamllint Outdated Show resolved Hide resolved
Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@mjnagel mjnagel left a comment

Choose a reason for hiding this comment

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

99% there, couple small things and then this should be g2g

@mjnagel mjnagel dismissed jeff-mccoy’s stale review March 6, 2024 21:59

Updates made based on requested changes.

@TristanHoladay TristanHoladay merged commit 196df88 into main Mar 7, 2024
15 checks passed
@TristanHoladay TristanHoladay deleted the feat-exemptions branch March 7, 2024 15:43
mjnagel pushed a commit that referenced this pull request Mar 7, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.15.0](v0.14.5...v0.15.0)
(2024-03-07)


### Features

* add policy exemptions
([#165](#165))
([196df88](196df88))


### Miscellaneous

* **deps:** update dependency defenseunicorns/uds-common to v0.2.2
([#232](#232))
([083ae0c](083ae0c))
* **deps:** update githubactions to de90cc6
([#215](#215))
([f79eed0](f79eed0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
robmcelvenny pushed a commit to owen-grady/uds-core-slim-dev that referenced this pull request Jun 3, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.15.0](defenseunicorns/uds-core@v0.14.5...v0.15.0)
(2024-03-07)


### Features

* add policy exemptions
([#165](defenseunicorns/uds-core#165))
([196df88](defenseunicorns/uds-core@196df88))


### Miscellaneous

* **deps:** update dependency defenseunicorns/uds-common to v0.2.2
([#232](defenseunicorns/uds-core#232))
([083ae0c](defenseunicorns/uds-core@083ae0c))
* **deps:** update githubactions to de90cc6
([#215](defenseunicorns/uds-core#215))
([f79eed0](defenseunicorns/uds-core@f79eed0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
rjferguson21 pushed a commit that referenced this pull request Jul 11, 2024
## Description
This PR adds a way for users to register policy exemptions at runtime
using the uds-core operator.

## Related Issue
Relates to #130 

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] 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/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request)
followed

---------

Co-authored-by: Micah Nagel <[email protected]>
rjferguson21 pushed a commit that referenced this pull request Jul 11, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.15.0](v0.14.5...v0.15.0)
(2024-03-07)


### Features

* add policy exemptions
([#165](#165))
([196df88](196df88))


### Miscellaneous

* **deps:** update dependency defenseunicorns/uds-common to v0.2.2
([#232](#232))
([083ae0c](083ae0c))
* **deps:** update githubactions to de90cc6
([#215](#215))
([f79eed0](f79eed0))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants