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

PWX-32732 : use portworx-restricted based on IsPrivileged flag #1191

Merged
merged 5 commits into from
Aug 9, 2023

Conversation

nikita-bhatia
Copy link
Contributor

@nikita-bhatia nikita-bhatia commented Aug 8, 2023

What this PR does / why we need it:
Do not use portworx-restricted SCC by default, use the old way and allow to enable/disable it via annotation

Which issue(s) this PR fixes (optional)
Closes # https://portworx.atlassian.net/browse/PWX-32732

Copy link
Collaborator

@zoxpx zoxpx left a comment

Choose a reason for hiding this comment

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

I think this PR will require some more work (see below)

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Patch coverage: 98.51% and no project coverage change.

Comparison is base (b330620) 80.03% compared to head (56dfb2e) 80.03%.

Additional details and impacted files
@@              Coverage Diff               @@
##           px-rel-23.7.0    #1191   +/-   ##
==============================================
  Coverage          80.03%   80.03%           
==============================================
  Files                 58       58           
  Lines              16580    16583    +3     
==============================================
+ Hits               13269    13272    +3     
  Misses              2379     2379           
  Partials             932      932           
Files Changed Coverage Δ
drivers/storage/portworx/component/lighthouse.go 93.43% <98.21%> (ø)
...rivers/storage/portworx/component/pvccontroller.go 91.50% <98.66%> (ø)
drivers/storage/portworx/component/csi.go 88.12% <100.00%> (+0.04%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@zoxpx zoxpx left a comment

Choose a reason for hiding this comment

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

This looks good -- thanks Nikita (one question logged below)

)
}

return k8sutil.CreateOrUpdateClusterRole(c.k8sClient, clusterRole)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be a new addition... was this a mistake, or was this line required ?

Copy link
Contributor Author

@nikita-bhatia nikita-bhatia Aug 9, 2023

Choose a reason for hiding this comment

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

This method was returning k8sutil.CreateOrUpdateClusterRole , moved it to last line to add conditional statement of SCC name before creating the cluster role.

@nikita-bhatia nikita-bhatia merged commit 75fc9b7 into px-rel-23.7.0 Aug 9, 2023
5 checks passed
nikita-bhatia added a commit that referenced this pull request Aug 9, 2023
* use portworx-restricted based on IsPrivileged flag

* Add UTs for install with non-privileged annotation

* Add UTs for install with non-privileged annotation

* remove debug logs
nikita-bhatia added a commit that referenced this pull request Aug 9, 2023
* PWX-32732 : use SCC based on IsPrivileged flag (#1191)

* use portworx-restricted based on IsPrivileged flag

* Add UTs for install with non-privileged annotation

* Add UTs for install with non-privileged annotation

* remove debug logs

* resolve conflict

* fix failing test
@piyush-nimbalkar piyush-nimbalkar deleted the PWX-32732 branch September 15, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants