Skip to content

Conversation

maltesander
Copy link
Member

@maltesander maltesander commented Dec 23, 2022

Description

New structure looks like this:

apiVersion: kafka.stackable.tech/v1alpha1 kind: KafkaCluster metadata: name: simple-kafka spec: image: productVersion: 3.3.1 stackableVersion: 0.3.0 clusterConfig: authentication: - authenticationClass: kafka-client-auth-tls tls: internalSecretClass: kafka-internal-tls serverSecretClass: tls zookeeperConfigMapName: simple-kafka-znode brokers: .... 

fixes: #529

test: https://ci.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/kafka-operator-it-custom/29/

Review Checklist

  • Code contains useful comments
  • CRD change approved (or not applicable)
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)
  • Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@maltesander maltesander added release-note/action-required Denotes a PR that introduces potentially breaking changes that require user action. changelog/crd-change labels Dec 23, 2022
@maltesander maltesander requested a review from a team December 23, 2022 15:20
@maltesander maltesander self-assigned this Dec 23, 2022
@maltesander maltesander marked this pull request as ready for review December 23, 2022 15:52
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

I really like the refactoring, many thanks!

@maltesander maltesander requested a review from sbernauer January 2, 2023 09:45
@maltesander maltesander requested a review from sbernauer January 2, 2023 10:03
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Sorry, this should be it 🙈

Copy link
Contributor

@vsupalov vsupalov left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@maltesander maltesander requested a review from sbernauer January 2, 2023 13:12
@maltesander
Copy link
Member Author

bors merge

bors bot pushed a commit that referenced this pull request Jan 2, 2023
# Description New structure looks like this: ``` apiVersion: kafka.stackable.tech/v1alpha1 kind: KafkaCluster metadata: name: simple-kafka spec: image: productVersion: 3.3.1 stackableVersion: 0.3.0 clusterConfig: authentication: - authenticationClass: kafka-client-auth-tls tls: internalSecretClass: kafka-internal-tls serverSecretClass: tls zookeeperConfigMapName: simple-kafka-znode brokers: .... ``` fixes: #529 test: https://ci.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/kafka-operator-it-custom/29/ Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
@bors
Copy link
Contributor

bors bot commented Jan 2, 2023

Pull request successfully merged into main.

Build succeeded!

And happy new year! 🎉

@bors bors bot changed the title Consolidate TLS encryption and authentication [Merged by Bors] - Consolidate TLS encryption and authentication Jan 2, 2023
@bors bors bot closed this Jan 2, 2023
@bors bors bot deleted the consolidate-encryption-and-authentication branch January 2, 2023 13:57
@maltesander
Copy link
Member Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 5, 2023

Already running a review

@sbernauer
Copy link
Member

🤔

bors bot pushed a commit to stackabletech/druid-operator that referenced this pull request Jan 10, 2023
# Description Fixes #365 Changes, heavily inspired by the [consolidation which recently happened for the kafka-operator](stackabletech/kafka-operator#532). Relates to stackabletech/issues#293 The new structure was guided by this snippet: ``` apiVersion: druid.stackable.tech/v1alpha1 kind: DruidCluster metadata: name: derby-druid spec: image: productVersion: 24.0.0 stackableVersion: 0.3.0 clusterConfig: authentication: - authenticationClass: druid-tls-authentication-class (tls) # String - authenticationClass: druid-ldap-authentication-class (ldap) # String authorization: opa: configMapName: test-opa package: druid zookeeperConfigMapName: druid-znode metadataStorageDatabase: dbType: derby connString: jdbc:derby://localhost:1527/var/druid/metadata.db;create=true host: localhost port: 1527 deepStorage: hdfs: configMapName: druid-hdfs directory: /druid tls: serverSecretClass: secret_class # Option<String>. *In general* defaults to "tls" internalSecretClass: secret_class # Option<String>. *In general* defaults to "tls" ``` ## Overview of introduced changes While working on the main issue, adjacent and somewhat-related refactorings/changes were introduced as well: * Prefer not to disable TLS for integration tests, where possible (justification: while the complexity is slightly higher, we are tested the recommended codepath more, as TLS is on by default) * Introduce dedicated authorization and security rust files * Adjustments to test helper scripts (mostly regarding uniformity and ergonomics) ## Highlight Security-validation logic is well tested! Co-authored-by: Vladislav Supalov <vladislav.supalov@stackable.de>
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.

3 participants