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(policy): add unsafe service protos and unsafe service proto Go gencode #1003

Merged
merged 9 commits into from
Jun 18, 2024

Conversation

jakedoublev
Copy link
Contributor

@jakedoublev jakedoublev commented Jun 18, 2024

First PR related to #115

@jakedoublev jakedoublev changed the title feat(policy): add unsafe service protos feat(policy): add unsafe service protos and unsafe service proto Go gencode Jun 18, 2024
@jakedoublev jakedoublev marked this pull request as ready for review June 18, 2024 16:20
@jakedoublev jakedoublev requested review from a team as code owners June 18, 2024 16:20
@jrschumacher
Copy link
Member

@jakedoublev I'm wondering if we want the client to return some value to verify they was to perform this destructive behavior. For instance if you want to delete you need to provide the id and the name.

@jrschumacher
Copy link
Member

Then we can just pass the user's input from CLI or web directly to the server. No need for user clients to add that layer.

@jakedoublev
Copy link
Contributor Author

jakedoublev commented Jun 18, 2024

@jakedoublev I'm wondering if we want the client to return some value to verify they was to perform this destructive behavior. For instance if you want to delete you need to provide the id and the name.

That's a good idea @jrschumacher. I think the behavior that would guard against is a mistaken UUID being utilized to delete the policy object. Some other options would be:

  1. requiring passage of parent/children (i.e. provide definition ids under the namespace when deleting a namespace, and values under a definition when deleting a definition)
  2. adding another param like acknowledgeRisk: boolean
  3. using the object fqn instead of its name/value to indicate an understanding of its place in the policy graph

Of those, I think 1 is too burdensome and 2 is awkward and clunky. Your suggestion is great to prevent a mistake and for consumer DX/UX downstream, and requiring the FQN instead of just the object name/value will require the user to know it, copy/paste, or type it in, which is good.

@jakedoublev
Copy link
Contributor Author

I think reactivation is okay with just an id because it is not cascading (like delete), which is most dangerous, and unsafe updates are likely also fine with just the id and the mutations.

The protos ensure gencode in each language and the HTTP path behind /unsafe will indicate the danger of these mutations.

@jakedoublev jakedoublev added this pull request to the merge queue Jun 18, 2024
Merged via the queue into main with commit 55cc045 Jun 18, 2024
16 checks passed
@jakedoublev jakedoublev deleted the feat/unsafe-policy branch June 18, 2024 22:43
github-merge-queue bot pushed a commit that referenced this pull request Jun 20, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.2.4](protocol/go/v0.2.3...protocol/go/v0.2.4)
(2024-06-18)


### Features

* **core:** New cryptoProvider config
([#939](#939))
([8150623](8150623))
* **policy:** add unsafe service protos and unsafe service proto Go
gencode ([#1003](#1003))
([55cc045](55cc045))


### Bug Fixes

* **core:** policy resource-mappings fix doc drift in proto comments
([#980](#980))
([09ab763](09ab763))

---
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: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Jun 24, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.4.7](service/v0.4.6...service/v0.4.7)
(2024-06-24)


### Features

* add dev_mode flag
([#985](#985))
([8da2436](8da2436))
* adds new trace log level
([#989](#989))
([25f699e](25f699e))
* Audit GetDecisions
([#976](#976))
([55bdfeb](55bdfeb))
* **authz:** Use flattened entity representations in subject mapping
evaluation ([#1007](#1007))
([b80443f](b80443f))
* **core:** add doublestar for public routes
([#998](#998))
([1c70c16](1c70c16))
* **core:** New cryptoProvider config
([#939](#939))
([8150623](8150623))
* **policy:** add unsafe service protos and unsafe service proto Go
gencode ([#1003](#1003))
([55cc045](55cc045))
* **policy:** policy unsafe namespace RPCs wired up to database
([#1018](#1018))
([239d9fa](239d9fa))
* **policy:** service stubs and registration for unsafe service
([#1009](#1009))
([9145491](9145491))


### Bug Fixes

* config loaded debug statement logs secrets
([#1010](#1010))
([6f6a603](6f6a603))
* **core:** Autobump service
([#1025](#1025))
([588827c](588827c))
* **core:** Fixes issue failing to find keys for kid-free kaos
([#982](#982))
([f27d484](f27d484))
* **core:** policy resource-mappings fix doc drift in proto comments
([#980](#980))
([09ab763](09ab763))
* **core:** Update to lib/fixtures 0.2.7
([#1017](#1017))
([dbae6ff](dbae6ff))
* **core:** Updates to protos 0.2.4
([#1014](#1014))
([43e11a3](43e11a3))
* **kas:** remove old logs
([#992](#992))
([192ff6d](192ff6d))

---
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: opentdf-automation[bot] <149537512+opentdf-automation[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.

2 participants