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: Azure plugin - remove Palette presets, reading permission set files #97

Merged
merged 6 commits into from
Jul 26, 2024

Conversation

mattwelke
Copy link
Member

@mattwelke mattwelke commented Jul 24, 2024

Issue

Resolves #84
Resolves #45

Description

Changes the plugin to not have Palette presets for Azure RBAC rules anymore.

Also changes it to not prompt the user for details for permission sets anymore. Instead, that data is read from a file the user provides. Permission sets are the portion of the RBAC rule consisting of the actions, data actions, and scope. The principal is also part of the rule but the user is prompted for that instead of it being read from a file. Most plugin users won't need more than one rule because while they may have multiple levels of scope to work with, they will likely only be validating one principal. This should be very minimal prompting, with most data coming from the file.

Example:

Note: You must configure at least one rule for plugin configuration.
Note: Collecting input for rule #1
Rule type: 
  > RBAC
Rule name: rule-1
Enter security principal.
Format: Azure GUID
Example: d6df0bba-800d-492f-802e-d04a38c80786
Security principal: cc7f25f8-4ab7-4274-a684-1f5670cd6c60
Note: You must configure at least one permission set for rule.
If you're updating an existing RBAC rule, its permission sets will be replaced.
Permission sets file path: tests/integration/_validator/testcases/data/azurePermissionSets.json
Add additional RBAC rule? [y/N]: No

This is intentional:

Rule type: 
  > RBAC

validatorctl only supports RBAC rules right now, but more rules are being added to the plugin and they will be added to validatorctl later. Leaving this here as a placeholder.

@mattwelke mattwelke requested a review from a team as a code owner July 24, 2024 18:07
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. bug Something isn't working labels Jul 24, 2024
@TylerGillson
Copy link
Member

@mattwelke is this still WIP? Please update the description and rebase if it's ready for review.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke
Copy link
Member Author

mattwelke commented Jul 24, 2024

@TylerGillson I'm going to refine this a bit. Instead of it prompting multiple times for permission set files until it has all the permission sets, I'm going to make a new JSON file format where users can specify all the permission sets at once. This would help in scenarios where they have more than one permission set because they require some permissions at scope x but other permissions only at scope y, and they don't want to over permission. And they'd be able to encode all of that in one file.

With the Azure plugin, while we can't make it so users can copy and paste JSON they find online (e.g. the JSON for a role they find on Azure' built-in roles page) until we have the new "role" rule we discussed offline, we can at least make it so that they have as few prompts to go through as possible because as much of the data as possible has been included in this JSON file.

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 41.46341% with 24 lines in your changes missing coverage. Please review.

@@            Coverage Diff             @@
##             main      #97      +/-   ##
==========================================
- Coverage   52.28%   50.81%   -1.48%     
==========================================
  Files          45       45              
  Lines        5009     4719     -290     
==========================================
- Hits         2619     2398     -221     
+ Misses       1729     1695      -34     
+ Partials      661      626      -35     
Files Coverage Δ
pkg/config/constants.go 100.00% <ø> (ø)
...integration/_validator/testcases/test_validator.go 94.43% <100.00%> (-0.48%) ⬇️
pkg/services/validator/azure.go 44.09% <38.46%> (-17.50%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b5326b...f7b3726. Read the comment docs.

@mattwelke mattwelke force-pushed the fix/azure-remove-palette-and-add-file-support branch from 6750ba3 to c9c0567 Compare July 24, 2024 21:58
@mattwelke
Copy link
Member Author

Made that update. Rebased. Ready for review.

@mattwelke mattwelke changed the title fix: Azure plugin - remove Palette presets, reading permission set files feat: Azure plugin - remove Palette presets, reading permission set files Jul 24, 2024
@mattwelke mattwelke added enhancement Enhancement to an existing feature and removed bug Something isn't working labels Jul 24, 2024
Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@mattwelke mattwelke force-pushed the fix/azure-remove-palette-and-add-file-support branch from e6ac592 to ca67e43 Compare July 25, 2024 15:58
@validator-labs validator-labs deleted a comment from gitguardian bot Jul 25, 2024
@mattwelke
Copy link
Member Author

GitGuardian comment was about fake secrets in test package accidentally accidentally committed. Cleaned up branch.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
Copy link

gitguardian bot commented Jul 25, 2024

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Signed-off-by: Matt Welke <matt.welke@spectrocloud.com>
@TylerGillson TylerGillson self-requested a review July 25, 2024 17:26
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 25, 2024
@TylerGillson TylerGillson merged commit 95787db into main Jul 26, 2024
7 of 8 checks passed
@TylerGillson TylerGillson deleted the fix/azure-remove-palette-and-add-file-support branch July 26, 2024 15:13
ahmad-ibra pushed a commit that referenced this pull request Jul 26, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.0.6](v0.0.5...v0.0.6)
(2024-07-26)


### Features

* Azure plugin - remove Palette presets, reading permission set files
([#97](#97))
([95787db](95787db))


### Other

* bump validator and plugin versions
([#106](#106))
([a3863aa](a3863aa))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
2 participants