Skip to content

[cherry-pick][CDAP-21258] Add FSGroupChangePolicy support to SecurityContext#167

Open
sidhdirenge wants to merge 1 commit into
release/2.7from
cherrypick-2.7-fsgroup
Open

[cherry-pick][CDAP-21258] Add FSGroupChangePolicy support to SecurityContext#167
sidhdirenge wants to merge 1 commit into
release/2.7from
cherrypick-2.7-fsgroup

Conversation

@sidhdirenge

Copy link
Copy Markdown
Contributor

Cherry-pick : #164

Add fsGroupChangePolicy support to the custom SecurityContext struct. This allows configuring fsGroupChangePolicy (e.g., OnRootMismatch) at the pod level for CDAP services like logs, significantly reducing pod startup latency on large volumes. Includes OpenAPI v3 validation enum and updated unit test cases.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for the FSGroupChangePolicy field in the SecurityContext of the CDAP operator, updating the API, templates, and test configurations. However, the changes to config/rbac/role.yaml severely restrict the RBAC permissions for configmaps and services to only create, which will cause the operator to fail during reconciliation. Full CRUD capabilities must be restored for these resources to function properly.

Comment thread config/rbac/role.yaml
Comment on lines 12 to 13
verbs:
- create

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The RBAC permissions for 'configmaps' and 'services' have been severely restricted to only 'create'. The operator requires full CRUD capabilities ('get', 'list', 'watch', 'create', 'update', 'patch', 'delete') for both resources to function properly (e.g., in 'ConfigMapHandler' and 'ServiceHandler'). This change will cause the operator to fail during reconciliation.

  verbs:
  - create
  - delete
  - get
  - list
  - patch
  - update
  - watch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The diff is combining configmaps and services into a single rule block containing all verbs; the other CRUD verbs are still in the file but hidden from the diff as unchanged context.

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.

1 participant