Skip to content

Conversation

@maltesander
Copy link
Member

Description

Spike for #792, https://github.com/stackabletech/decisions/issues/66

This consolidates the operators authorization part closer to the nifi internals:

image

The authorization part consisted of an OPA and Default part. The Default part was a mix out of the SingleUser authorizer (e.g. for SingleUser or OIDC authentication) as well as file-based for LDAP authentication.
Filebased changes to users or authorizations were not persisted properly (ephermeral) which lead to problems.

This is now consolidated according to the diagram above:

spec:
  clusterConfig:
    authorization: # complex enum, defaults to `singleUser: {}`, so should be non-breaking (I would need to look at the generated CRD to be sure)
      opa: ... # existing
      singleUser: {} # new
      standard: #  new -> this internally adds a PVC to the STS
        accessPolicyProvider:
          fileBased:
            initialAdminUser: CN=admin,OU=admin-group,DC=example,DC=org

This is breaking for LDAP users that now explicitly have to set the standard authorization method and an initial admin user.

The PVC provided size for the filebased authorization is currently fixed to 16MB and cannot be configured other than pod overrides.

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

  • Changes are OpenShift compatible
  • CRD changes approved
  • CRD documentation for all fields, following the style guide.
  • Helm chart can be installed and deployed operator works
  • Integration tests passed (for non trivial changes)
  • Changes need to be "offline" compatible
  • Links to generated (nightly) docs added
  • Release note snippet added

Reviewer

  • Code contains useful comments
  • Code contains useful logging statements
  • (Integration-)Test cases added
  • Documentation added or updated. Follows the style guide.
  • Changelog updated
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

  • Feature Tracker has been updated
  • Proper release label has been added
  • Links to generated (nightly) docs added
  • Release note snippet added
  • Add type/deprecation label & add to the deprecation schedule
  • Add type/experimental label & add to the experimental features tracker

@maltesander maltesander self-assigned this Jan 11, 2026
@maltesander maltesander added release-note Denotes a PR that will be considered when it comes time to generate release notes. release-note/action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Jan 11, 2026
@maltesander maltesander moved this to Development: Waiting for Review in Stackable Engineering Jan 11, 2026
@dervoeti dervoeti self-requested a review January 13, 2026 08:10
Copy link
Member

@dervoeti dervoeti left a comment

Choose a reason for hiding this comment

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

Nice, LGTM overall, just some nits.

We should provide migration instructions in the release notes for users that use LDAP.

resources: Some(VolumeResourceRequirements {
requests: Some({
let mut map = BTreeMap::new();
map.insert("storage".to_string(), Quantity("16Mi".to_owned()));
Copy link
Member

Choose a reason for hiding this comment

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

Maybe make this a constant? Just an idea though, feel free to disregard. Like:

const FILE_BASED_STORAGE_SIZE: &str = "16Mi";

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah i can do that, however i was not quite sure how to size it. Feels fine for "normal" setups, but you can easily set permissions on a very granular level (or have just many many users) to blow up its size...

My preference would be to make it even configurable (without pod overrides) but i could not think of a clean solution.

Copy link
Member

Choose a reason for hiding this comment

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

I mean we could do something like:

spec:
  clusterConfig:
    authorization:
      standard:
        accessPolicyProvider:
          fileBased:
            initialAdminUser: CN=admin,OU=admin-group,DC=example,DC=org
            storage:
              capacity: 1Gi # default to 16Mi

We can also leave it at 16Mi hardcoded for now, but if it's not unlikely to cause problems in production at some point we could try to make it configurable. I can't really judge that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah i like that but if we ever want to provide a UserGroupProvider where does it go :D ? It is a bit intertwined.

@dervoeti dervoeti moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Jan 13, 2026
@maltesander maltesander requested a review from dervoeti January 13, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/action-required Denotes a PR that introduces potentially breaking changes that require user action. release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

Status: Development: In Review

Development

Successfully merging this pull request may close these issues.

3 participants