Skip to content

Let secret-op handle pkcs12 stores#505

Merged
dervoeti merged 11 commits intomainfrom
let-secret-op-create-pkcs12-stores
Sep 26, 2023
Merged

Let secret-op handle pkcs12 stores#505
dervoeti merged 11 commits intomainfrom
let-secret-op-create-pkcs12-stores

Conversation

@dervoeti
Copy link
Member

@dervoeti dervoeti commented Sep 11, 2023

Description

Fixes #502.

Keytool is used to set a password for the PKCS12 stores, because NiFi complains when no password is specified:

WARN [main] o.a.nifi.security.util.SslContextFactory Some truststore properties are populated (/stackable/server_tls/truststore.p12, ********, PKCS12) but not valid
Failed to start web server: Invalid nifi.web.https configuration in nifi.properties

At least I could not make it work without a password for the store.

Local Kuttl test for LDAP with TLS worked fine.

Not sure about the naming of STACKABLE_SERVER_TLS_DIR, I just used the same name that was used in the change for trino-operator. Might not be appropiate for NiFi.

Also: I don't think we need the random destination alias (as implemented in trino-operator) in this case, but please re-check this.

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
- [x] Changes are OpenShift compatible
- [x] CRD changes approved
- [x] Helm chart can be installed and deployed operator works
- [ ] Integration tests passed (for non trivial changes)
- [ ] Changes need to be "offline" compatible
# Reviewer
- [x] Code contains useful comments
- [x] (Integration-)Test cases added
- [x] Documentation added or updated
- [x] Changelog updated
- [x] 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

@dervoeti
Copy link
Member Author

When stackabletech/secret-operator#314 is merged, we can probably get rid of the two keytool commands. I think the emptyDir volume is still needed, since an LDAP tls certificate might be added to the truststore (we should not write to the secret-op mount). But we could simply cp truststore.p12 to the emptyDir volume.

@maltesander maltesander self-requested a review September 13, 2023 14:40
Copy link
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

When stackabletech/secret-operator#314 is merged, we can probably get rid of the two keytool commands. I think the emptyDir volume is still needed, since an LDAP tls certificate might be added to the truststore (we should not write to the secret-op mount). But we could simply cp truststore.p12 to the emptyDir volume.

Agreed, after operator-rs is released we can just copy. We need the password annotation though otherwise we cannot add the ldap cert.

@dervoeti
Copy link
Member Author

@maltesander Thank you very much for the feedback, implemented all your suggestions. I also found a bug in the "create reporting task" job, which is a python script that needs the CA certificate to verify the server's identity. The certificate itself is not present anymore and has to be extracted from the PKCS12 store, which is now handled by keytool before the script runs.

@maltesander
Copy link
Member

@maltesander Thank you very much for the feedback, implemented all your suggestions. I also found a bug in the "create reporting task" job, which is a python script that needs the CA certificate to verify the server's identity. The certificate itself is not present anymore and has to be extracted from the PKCS12 store, which is now handled by keytool before the script runs.

I think we can just mount the cert here directly since its an independent job/pod?

@dervoeti dervoeti force-pushed the let-secret-op-create-pkcs12-stores branch from bc3bf6b to 272ee46 Compare September 24, 2023 20:39
Copy link
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@dervoeti dervoeti added this pull request to the merge queue Sep 26, 2023
Merged via the queue into main with commit ba9b04d Sep 26, 2023
@dervoeti dervoeti deleted the let-secret-op-create-pkcs12-stores branch September 26, 2023 06:45
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.

Let secret-operator handle PKCS#12 conversion

2 participants