strimzi / kafka-access-operator

Operator for sharing access to Strimzi clusters across namespaces
Apache License 2.0
14 stars 13 forks source link

feat: Add event sources and create secret during reconcile #2

Closed katheris closed 2 years ago

katheris commented 2 years ago

Signed-off-by: Katherine Stanley 11195226+katheris@users.noreply.github.com

katheris commented 2 years ago

Hey @ppatierno, thanks for your review. Some thoughts on your general comments:

Looking a the proposal again while reviewing this, there is no references about the expected labels we are going to have on the Secret that we use to filter events in this operator. Maybe it would be useful to integrate the proposal with such information?

This isn't specifically listed in the proposal at the moment, but the label I have chosen managed-by is one of the Kubernetes recommended labels (https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/) and is used in the Strimzi operator already so I think it is ok not to call it out specifically in the proposal.

About the operator watching Kafka and Secret operands as well and how. Wdyt?

The proposal did call out that it would watch the Kafka and Secret operands (step 11 of the implementation https://github.com/strimzi/proposals/blob/main/033-service-binding.md#implementation). In terms of how it does it, there was some discussion on the proposal about how much or little it should cover implementation and in general I think we didn't going into specific implementation details.

Is the reason you are asking because it could be unclear to others that are seeing the code for the first time? At the moment the java-operator-sdk documentation is limited, do you think it would be useful to have a paragraph in the README.md that explains which resources the operator watches and why?

ppatierno commented 2 years ago

@katheris Yeah I think that some info on the README could be a good idea ;-)