Skip to content

[Merged by Bors] - Add support for secret-operator volumes to pod volume builder #342

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

Closed
wants to merge 1 commit into from

Conversation

nightkr
Copy link
Member

@nightkr nightkr commented Mar 7, 2022

Description

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)

@nightkr nightkr requested review from sbernauer and a team March 7, 2022 15:49
@nightkr nightkr self-assigned this Mar 7, 2022
@sbernauer
Copy link
Member

Thanks a lot!
In the AuthenticationClass we get a user-provided scope from the user (see below). My understanding is, that the attribute spec.protocol.ldap.bindCredentials.scope is a string and is just passed to the Pod volume, is that correct?
In that case we would need a way to just pass in the secrets.stackable.tech/scope string and skip creating Rust vectors.

apiVersion: authentication.stackable.tech/v1alpha1
kind: AuthenticationClass
metadata:
  name: myldap
spec:
  protocol:
    ldap:
      hostname: ldap.server
      port: 389
      domain: domain.local
      bindCredentials:
        secretClass: superset-with-ldap-ldap-bind
        scope: Service

@nightkr
Copy link
Member Author

nightkr commented Mar 7, 2022

My understanding is, that the attribute spec.protocol.ldap.bindCredentials.scope is a string and is just passed to the Pod volume, is that correct?

Sort of.. the question there would be how much we want to validate in the operator vs just passing it through. We might also want to force certain scopes when we know that a service will require them.. not sure there.

@sbernauer
Copy link
Member

Alright, i will think of a solution

@sbernauer
Copy link
Member

I will also add some unit test as I'm working on new features for SecretOperatorVolumeSourceBuilder.
So I'm going to check that in the Review Checklist

@nightkr
Copy link
Member Author

nightkr commented Mar 8, 2022

Kinda getting mixed signals here... :P

@sbernauer
Copy link
Member

I'm happy to merge this as is. I'm working on the "passing a scope string to SecretOperatorVolumeSourceBuilder" but will open a separate PR for that.

@nightkr
Copy link
Member Author

nightkr commented Mar 8, 2022

Alright.

bors r+

bors bot pushed a commit that referenced this pull request Mar 8, 2022
## Description

## Review Checklist
- [x] Code contains useful comments
- [x] (Integration-)Test cases added (or not applicable)
- [x] Documentation added (or not applicable)
- [x] Changelog updated (or not applicable)


Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
@bors
Copy link
Contributor

bors bot commented Mar 8, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Add support for secret-operator volumes to pod volume builder [Merged by Bors] - Add support for secret-operator volumes to pod volume builder Mar 8, 2022
@bors bors bot closed this Mar 8, 2022
@bors bors bot deleted the feature/secret-op-builder branch March 8, 2022 08:27
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.

2 participants