Skip to content

CLOUDP-331496: Remove undocumented operator.enablePVCResize Helm value #272

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m1kola
Copy link
Contributor

@m1kola m1kola commented Jul 16, 2025

Summary

In #260 I moved PersistentVolumeClaim permissions into a separate role, enabling modularity because these permissions are controlled by operator.enablePVCResize Helm value and appeared to be optional.

Later I noticed that operator.enablePVCResize is undocumented.

I propose that we revert changes done in #260 and remove the undocumented setting. Permissions are enabled by default already and I do not think there is a rationale for this to be configurable.

Furthermore - we do not test this with operator.enablePVCResize set to false. We don't even know if the operator will still function correctly without the permissions.

Proof of Work

N/A - CI must be green.

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you checked for release_note changes?

Reminder (Please remove this when merging)

  • Please try to Approve or Reject Changes the PR, keep PRs in review as short as possible
  • Our Short Guide for PRs: Link
  • Remember the following Communication Standards - use comment prefixes for clarity:
    • blocking: Must be addressed before approval.
    • follow-up: Can be addressed in a later PR or ticket.
    • q: Clarifying question.
    • nit: Non-blocking suggestions.
    • note: Side-note, non-actionable. Example: Praise
    • --> no prefix is considered a question

@m1kola m1kola marked this pull request as ready for review July 17, 2025 08:06
@m1kola m1kola requested a review from a team as a code owner July 17, 2025 08:06
Copy link
Collaborator

@MaciejKaras MaciejKaras left a comment

Choose a reason for hiding this comment

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

Agree, let's remove the undocumented field.

@lsierant lsierant requested a review from nammn July 21, 2025 08:09
@@ -4,7 +4,7 @@
# MCK 1.3.0 Release Notes

## Other Changes
* Optional permissions for `PersistentVolumeClaim` moved to a separate role. When managing the operator with Helm it is possible to disable permissions for `PersistentVolumeClaim` resources by setting `operator.enablePVCResize` value to `false` (`true` by default). When enabled, previously these permissions were part of the primary operator role. With this change, permissions have a separate role.
* The undocumented `operator.enablePVCResize` Helm value has been removed. If you previously set this value to `false`, please note that the operator roles will now include permissions for `PersistentVolumeClaim` resources by default.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be considered a breaking change, so requiring bumping major version...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arguably it is not a breaking change since we are adding new RBAC by default. Yes, we are removing the setting which only appears in values.yaml, but at runtime it won't negatively affect the operator or workloads.

Copy link
Contributor

@lsierant lsierant left a comment

Choose a reason for hiding this comment

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

Removing this switch should be considered as a breaking change.

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