Skip to content
This repository has been archived by the owner on Oct 18, 2024. It is now read-only.

docs(mtls): add example in combination with 'KafkaTopic' and 'KafkaUser' #231

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sebastiangaiser
Copy link

closes #230

packaging/examples/mtls/mtls.yaml Outdated Show resolved Hide resolved
packaging/examples/mtls/mtls.yaml Show resolved Hide resolved
packaging/examples/mtls/mtls.yaml Outdated Show resolved Hide resolved
packaging/examples/mtls/mtls.yaml Outdated Show resolved Hide resolved
@ppatierno ppatierno added this to the 0.7.0 milestone Apr 30, 2024
Co-authored-by: Paolo Patierno <[email protected]>
Signed-off-by: Sebastian Gaiser <[email protected]>
@ppatierno ppatierno requested a review from scholzj April 30, 2024 14:07
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

  • Maybe we should add some README.md into the folder with some explanation?
  • Maybe we should make it in the path a bit more clear that this is about linking it with Strimzi-based Kafka cluster and not as muhc about mTLS as the path suggests? E.g. packaging/examples/strimzi-tls-auth?
  • It seems disconnected from the release tooling. So if we ever do another release of this, nothing will hapen with this. So I guess the MAkefile tasks should be updated to:
    • Include this in the release package
    • Copy it to /examples
    • Make sure the Deployment YAML is up to date (I think having a separate deployment is a bit controversial as it would get easily out of date. But not sure how to do it better other thn having just a README.md with YAML snippets.)
    • Update the Deployment YAML during release with the release version (to use the correct container)

I guess having this just as a section in the main README.md would safe a lot of these things. But I'm fine if we want to have it in examples. We just have to make sure it is done properly or it will be obsolete and forgotten soon.

@sebastiangaiser
Copy link
Author

If you want I can move the files to a different path, split them up, ... Please tell me where :D

The structure of the readme and the makefile parts are not on me, I guess. So maybe this should be in another PR.

@scholzj
Copy link
Member

scholzj commented May 2, 2024

If you want I can move the files to a different path, split them up, ... Please tell me where :D

The structure of the readme and the makefile parts are not on me, I guess. So maybe this should be in another PR.

I think there are several problems with this:

  • You do not really have an example. You have installation files.
    • Installation files need to be properly maintained (e.g. if you add a new environment variable you have to add it to all instances).
    • Installation files need to be package as part of the release
    • Installation files need to be updated as part of the release (if nothing else, the image tag changes)

If we merge it as it is, it will never be part of any release, most people will never find it and it will eventually get outdated until it is one day deleted. That is what I'm afraid of.

@sebastiangaiser
Copy link
Author

Would the releasing be easier with a Helm chart? I think this would not be that kind of documentation but in case when you run helm template... you would also get the "raw" manifests, too.

@scholzj
Copy link
Member

scholzj commented May 12, 2024

Would the releasing be easier with a Helm chart? I think this would not be that kind of documentation but in case when you run helm template... you would also get the "raw" manifests, too.

Well, yes and not I guess. We would still need to integrate it into the tooling and CIs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide docs using with strimzi-kafka-operator
3 participants