Skip to content

Conversation

@bznein
Copy link
Contributor

@bznein bznein commented Sep 18, 2020

This PR changes the default permissions on volumes created from a secret. This is needed for another work in the enterprise operator. It will still be possible to specify different modes through the yaml file (when the merging bug is fixed)

All Submissions:

  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

@bznein bznein requested a review from chatton September 18, 2020 08:17
Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

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

Looks good to me, as discussed in person we should add some unit tests to check the merging logic around this field.

@bznein
Copy link
Contributor Author

bznein commented Sep 18, 2020

@chatton as discussed in person, putting a simple unit test for now and then I'll add a more proper tests around merging when fixing it. Ready for another review when you have time!

Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@bznein bznein merged commit a1b7305 into master Sep 18, 2020
@bznein bznein deleted the change_default_permissions branch September 18, 2020 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants