feat(main): refactor actions within existing CLI policy object CRUD #543
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the CLI’s handling of subject mappings to support the new CRUDable actions while deprecating the old proto flag approach. Key changes include:
- Passing context.Context to all subject mapping handler functions.
- Renaming parameters (e.g. attrValId to attrValID) for consistency.
- Updating CLI commands and documentation to replace separate standard and custom action flags with a unified --action flag.
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/handlers/subjectmappings.go | Updated handler functions to receive a context parameter and improved parameter naming consistency. |
| docs/man/policy/subject-mappings/*.md | Updated documentation to reflect the new action flag and deprecate the old flags. |
| docs/man/policy/attributes/create.md | Clarified propagation of actions within attribute mappings. |
| cmd/policy-subjectMappings.go | Refactored CLI command functions to work with the new context passing and unified action flag approach, with deprecated flags maintained for backward compatibility. |
Files not reviewed (3)
- e2e/encrypt-decrypt.bats: Language not supported
- e2e/subject-condition-sets.bats: Language not supported
- e2e/subject-mapping.bats: Language not supported
Comments suppressed due to low confidence (2)
cmd/policy-subjectMappings.go:341
- The registration of the deprecated '--action-standard' flag uses a temporary slice literal (&[]string{}). Even if the flag is deprecated, consider assigning it to a dedicated variable to ensure clarity and preserve flag value integrity if it ever needs to be referenced.
createDoc.Flags().StringSliceVarP(&[]string{},
cmd/policy-subjectMappings.go:387
- Similarly, the deprecated '--action-custom' flag in the update command is registered with a temporary slice literal (&[]string{}). It is recommended to use a dedicated variable for these deprecated flags for better maintainability.
updateDoc.Flags().StringSliceVarP(&[]string{},
ryanulit
approved these changes
Apr 25, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the CLI handling of subject mapping operations to use new CRUDable actions and context propagation while updating the associated documentation. Key changes include:
- Refactoring handler methods to accept a context parameter instead of using a stored context.
- Replacing and deprecating the old "--action-standard" and "--action-custom" flags with the new "--action" flag across the CLI commands.
- Updating documentation to clearly describe the new subject mapping actions and behavior.
Reviewed Changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/handlers/subjectmappings.go | Updated functions to use context and improved parameter naming consistency. |
| docs/man/policy/subject-mappings/update.md | Modified flag descriptions and marked old flags as deprecated. |
| docs/man/policy/subject-mappings/create.md | Adjusted instructions/examples to use the new '--action' flag. |
| docs/man/policy/subject-mappings/_index.md | Updated text to reflect subject mapping actions changes. |
| docs/man/policy/attributes/create.md | Revised description for attribute entitlement actions. |
| cmd/policy-subjectMappings.go | Renamed command functions, updated context usage, and refactored flag logic. |
Files not reviewed (3)
- e2e/encrypt-decrypt.bats: Language not supported
- e2e/subject-condition-sets.bats: Language not supported
- e2e/subject-mapping.bats: Language not supported
jakedoublev
added a commit
that referenced
this pull request
Jun 3, 2025
🤖 I have created a release *beep* *boop* --- ## [0.21.0](v0.20.0...v0.21.0) (2025-05-29) ### Features * Add initial Dependency Review configuration ([#551](#551)) ([b622666](b622666)) * **core:** Add base key cmds ([#563](#563)) ([edfd6c0](edfd6c0)) * **core:** DSPX-18 clean up Go context usage to follow best practices ([#558](#558)) ([a2c9f8b](a2c9f8b)) * **core:** DSPX-608 - Deprecate public_client_id ([#555](#555)) ([8d396bd](8d396bd)) * **core:** DSPX-608 - require clientID for login ([#553](#553)) ([580172e](580172e)) * **core:** DSPX-896 add registered resources CRUD ([#559](#559)) ([8e7475e](8e7475e)) * **core:** KAS allowlist options ([#539](#539)) ([af7978f](af7978f)) * **core:** key management operations ([#533](#533)) ([d4f6aaa](d4f6aaa)) * **main:** add actions CRUD and e2e tests ([#523](#523)) ([2fb9ec7](2fb9ec7)) * **main:** refactor actions within existing CLI policy object CRUD ([#543](#543)) ([9ab1a58](9ab1a58)) * **core:** Resource mapping groups ([#567](#567)) ([03fa307](03fa307)) * **core:** Update key mgmt flags to consistent format ([#570](#570)) ([#846f96c](846f96c)) * **core:** Rotate Key ([#572](#572)) ([afd0043](afd0043)) ### Bug Fixes * **ci:** ci job should run on changes to GHA ([#530](#530)) ([1d296ca](1d296ca)) * **main:** Pass the full url when building the sdk object ([#544](#544)) ([8b836f0](8b836f0)) --- 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> Co-authored-by: Elizabeth Healy <[email protected]> Co-authored-by: Chris Reed <[email protected]> Co-authored-by: Jake Van Vorhis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates to use new CRUDable actions rather than proto flags: