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!: support additional vCenter entities for privilege rules #362

Merged
merged 27 commits into from
Sep 10, 2024

Conversation

TylerGillson
Copy link
Member

@TylerGillson TylerGillson commented Sep 6, 2024

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:

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson TylerGillson requested a review from a team as a code owner September 6, 2024 21:49
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement Enhancement to an existing feature labels Sep 6, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
ahmad-ibra
ahmad-ibra previously approved these changes Sep 6, 2024
Copy link
Contributor

@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.

LGTM aside from the minor grammar issue. Also thanks for cleaning up all the helper functions that weirdly returned a bool along with the err and actual return value. It always felt like some weird anti-pattern.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 6, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Copy link

codecov bot commented Sep 7, 2024

Codecov Report

Attention: Patch coverage is 32.26381% with 380 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/vsphere/user.go 0.00% 150 Missing ⚠️
pkg/vsphere/network.go 50.87% 47 Missing and 9 partials ⚠️
pkg/vsphere/datacenter.go 0.00% 31 Missing ⚠️
pkg/vsphere/cluster.go 0.00% 30 Missing ⚠️
pkg/vsphere/vsphere_mocks.go 0.00% 20 Missing ⚠️
pkg/vsphere/folder.go 0.00% 19 Missing ⚠️
pkg/vsphere/vm.go 60.97% 16 Missing ⚠️
pkg/vsphere/datastore.go 50.00% 9 Missing and 4 partials ⚠️
pkg/vsphere/host.go 23.07% 10 Missing ⚠️
pkg/vsphere/vapp.go 0.00% 6 Missing ⚠️
... and 10 more
@@            Coverage Diff             @@
##             main     #362      +/-   ##
==========================================
+ Coverage   38.75%   39.20%   +0.45%     
==========================================
  Files          29       32       +3     
  Lines        1729     1977     +248     
==========================================
+ Hits          670      775     +105     
- Misses        987     1106     +119     
- Partials       72       96      +24     
Files with missing lines Coverage Δ
api/v1alpha1/vspherevalidator_types.go 44.82% <ø> (ø)
pkg/validators/privileges/privilege.go 100.00% <100.00%> (ø)
tests/integration/tags/tags.go 88.50% <100.00%> (ø)
...kg/validators/computeresources/computeresources.go 76.64% <95.00%> (+0.60%) ⬆️
pkg/vcsim/vcsim.go 88.23% <95.23%> (+1.71%) ⬆️
api/vcenter/types.go 50.00% <0.00%> (-38.89%) ⬇️
pkg/validators/ntp/ntp.go 18.51% <0.00%> (-0.72%) ⬇️
pkg/vsphere/tag.go 8.47% <0.00%> (ø)
pkg/validate/validate.go 40.54% <57.14%> (-3.75%) ⬇️
pkg/vsphere/resource_pool.go 0.00% <0.00%> (ø)
... and 13 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 37af6b3...d5272af. Read the comment docs.

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
…l tag & name filtering, clean up vcsim

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
…privilege

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson TylerGillson changed the title feat: support additional vCenter entities for privilege rules feat!: support additional vCenter entities for privilege rules Sep 10, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
ahmad-ibra
ahmad-ibra previously approved these changes Sep 10, 2024
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
@TylerGillson TylerGillson merged commit abe3a94 into main Sep 10, 2024
7 checks passed
@TylerGillson TylerGillson deleted the feat/additional-entities branch September 10, 2024 20:26
TylerGillson added a commit that referenced this pull request Sep 10, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](v0.0.34...v0.1.0)
(2024-09-10)


### ⚠ BREAKING CHANGES

* support additional vCenter entities for privilege rules
([#362](#362))
* remove RolePrivilegeValidationRules, add enums to API, remove "cloud"
refs and simplify account handling
([#357](#357))

### Features

* support additional vCenter entities for privilege rules
([#362](#362))
([abe3a94](abe3a94))


### Docs

* fix typos
([f9b63d8](f9b63d8))
* update CR samples
([#367](#367))
([e6968ba](e6968ba))


### Dependency Updates

* **deps:** update golang.org/x/exp digest to 701f63a
([#364](#364))
([37af6b3](37af6b3))
* **deps:** update golang.org/x/exp digest to e7e105d
([#355](#355))
([b67befa](b67befa))
* **deps:** update module github.com/onsi/ginkgo/v2 to v2.20.2
([#353](#353))
([f9eab82](f9eab82))
* **deps:** update module github.com/onsi/gomega to v1.34.2
([#354](#354))
([d834600](d834600))
* **deps:** update module github.com/validator-labs/validator to v0.1.10
([#356](#356))
([3c0c928](3c0c928))
* **deps:** update module github.com/validator-labs/validator to v0.1.9
([#347](#347))
([cd8ff75](cd8ff75))
* **deps:** update module sigs.k8s.io/cluster-api to v1.8.2
([#358](#358))
([0f7c799](0f7c799))


### Refactoring

* remove RolePrivilegeValidationRules, add enums to API, remove "cloud"
refs and simplify account handling
([#357](#357))
([4388804](4388804))
* rename CloudDriver -&gt; VCenterDriver
([#361](#361))
([8943ff6](8943ff6))
* vCenter entity type constants
([#360](#360))
([3fb6f51](3fb6f51))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
TylerGillson added a commit to validator-labs/validatorctl that referenced this pull request Sep 11, 2024
## 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:
- validator-labs/validator-plugin-vsphere#362

---------

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
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:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants