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

refactor!: remove role privilege rules #217

Merged
merged 18 commits into from
Sep 11, 2024
Merged

refactor!: remove role privilege rules #217

merged 18 commits into from
Sep 11, 2024

Conversation

TylerGillson
Copy link
Member

@TylerGillson TylerGillson commented Sep 5, 2024 •
edited
Loading

Issue

N/A

Description

  • Remove RolePrivilegeRules
  • Add support for privilege rules on all entities
  • Configure GroupPrincipals and Propagated for privilege rules
  • Bump golangci-lint and fix import issues related to _validator

Requires:

@TylerGillson
refactor: remove role privilege rules
743cf13 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: fix lints
8426d56 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: Merge branch 'main' into refactor/remove-role-privilege-rules
cf40147
@TylerGillson
chore: bump vSphere plugin
908dca6 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson TylerGillson marked this pull request as ready for review September 9, 2024 15:21
@TylerGillson TylerGillson requested a review from a team as a code owner September 9, 2024 15:21
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 9, 2024
@TylerGillson TylerGillson mentioned this pull request Sep 9, 2024
Merged
@dosubot dosubot bot added the refactoring Refactoring / tech debt label Sep 9, 2024
ahmad-ibra
ahmad-ibra previously approved these changes Sep 9, 2024
View reviewed changes
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 9, 2024
@TylerGillson
feat: handle privileges for all entities, propagation, and group prin…
446db57 
…cipals

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 10, 2024
@TylerGillson
chore: Merge branch 'main' into refactor/remove-role-privilege-rules
2d99cfa
@TylerGillson
chore: fix typo
337e425 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
feat: handle propagation properly
cdf6550 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: make reviewable
a43ee9f 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: fix test
5afc989 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: make reviewable
5349edf 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
test: fix integration test data
89a87f6 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: ensure int. tests pass w/ if local AWS creds file exists; tidy…
e2e78d1 
… AWS code

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
fix: handle vSphere API changes
7908bfc 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Sep 10, 2024
@TylerGillson
fix: exit early if propagation disabled
ce5faa6 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
TylerGillson added a commit to validator-labs/validator-plugin-vsphere that referenced this pull request Sep 10, 2024
@TylerGillson
feat!: support additional vCenter entities for privilege rules (#362)
abe3a94 
## Issue
N/A

## Description
- Extend privilege rules to support datastores, the vCenter root object,
and all network types.
- Add `GroupPrincipals` and `Propagated` to the privilege rule spec.
- `GroupPrincipals` are used to identify permissions that grant
privileges to a user on a specific entity. They're required because
non-admin users cannot query the vCenter API to determine their own
group membership.
- `Propagated` is a new flag that further qualifies the assignment of
privileges to a user on a specific entity.
- Clean up some lingering Spectro-internal logic that was filtering
Datacenters and Clusters based on kubernetes tags.

Related:
- validator-labs/validatorctl#217

---------

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
chore: make reviewable
982560b 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
ci: fix update-versions.sh; make reviewable
4f3778b 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson
fix: bump vSphere plugin & address API changes; fix int. tests
8b506c2 
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@codecov Codecov
Copy link

codecov bot commented Sep 11, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 46.32035% with 124 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/services/validator/vmware.go 38.01% 85 Missing and 21 partials ⚠️
pkg/services/validator/aws.go 18.18% 5 Missing and 4 partials ⚠️
pkg/services/clouds/aws_service.go 62.50% 3 Missing ⚠️
pkg/services/clouds/vmware_service.go 62.50% 2 Missing and 1 partial ⚠️
pkg/components/validator.go 66.66% 0 Missing and 2 partials ⚠️
...tegration/validatorctl/testcases/test_validator.go 95.65% 0 Missing and 1 partial ⚠️ 
@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
- Coverage   53.43%   53.30%   -0.13%     
==========================================
  Files          46       46              
  Lines        6367     6339      -28     
==========================================
- Hits         3402     3379      -23     
- Misses       2105     2114       +9     
+ Partials      860      846      -14
Files with missing lines Coverage Δ
pkg/cmd/validator/validator.go 60.07% <100.00%> (ø)
pkg/config/constants.go 100.00% <ø> (ø)
pkg/services/validator/rule_names.go 42.85% <ø> (ø)
tests/integration/validatorctl/executor.go 100.00% <ø> (ø)
tests/utils/file/directory.go 71.42% <100.00%> (ø)
...tegration/validatorctl/testcases/test_validator.go 93.35% <95.65%> (ø)
pkg/components/validator.go 56.44% <66.66%> (ø)
pkg/services/clouds/aws_service.go 60.49% <62.50%> (-1.16%) ⬇️
pkg/services/clouds/vmware_service.go 44.64% <62.50%> (ø)
pkg/services/validator/aws.go 50.22% <18.18%> (+<0.01%) ⬆️
... and 1 more

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 c4ac163...8b506c2. Read the comment docs.

ahmad-ibra
ahmad-ibra approved these changes Sep 11, 2024
View reviewed changes
Copy link
Collaborator

@ahmad-ibra ahmad-ibra left a comment

Choose a reason for hiding this comment

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

Left a question which may result in changes, but everything looks good to me

@TylerGillson TylerGillson merged commit b0976db into main Sep 11, 2024
7 of 8 checks passed
@TylerGillson TylerGillson deleted the refactor/remove-role-privilege-rules branch September 11, 2024 16:38
@TylerGillson TylerGillson mentioned this pull request Sep 9, 2024
Merged
TylerGillson added a commit that referenced this pull request Sep 11, 2024
@TylerGillson
chore(main): release 0.2.0 (#213)
180f0e3 
🤖 I have created a release *beep* *boop*
---


##
[0.2.0](v0.1.4...v0.2.0)
(2024-09-11)


### ⚠ BREAKING CHANGES

* remove role privilege rules
([#217](#217))

### Features

* add --custom-resources flag to validator rules check as an alternate
to -f
([#218](#218))
([4aa14b6](4aa14b6))


### Bug Fixes

* add missing Host.Config.Storage privilege & document
([#212](#212))
([c7408a6](c7408a6))


### Docs

* remove subcommand docs from repo
([#205](#205))
([57ca6cb](57ca6cb))


### Dependency Updates

* **deps:** update golang.org/x/exp digest to e7e105d
([#214](#214))
([c4ac163](c4ac163))


### Refactoring

* remove role privilege rules
([#217](#217))
([b0976db](b0976db))
* update `executePlugins` function to operate on a slice of
`PluginSpec`'s
([#206](#206))
([97875f3](97875f3))

---
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
Reviewers

@ahmad-ibra ahmad-ibra ahmad-ibra approved these changes

Assignees
No one assigned
Labels
lgtm This PR has been approved by a maintainer refactoring Refactoring / tech debt size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TylerGillson @ahmad-ibra