Skip to content

Conversation

@clintfred
Copy link

…(default port 31111).

Added a layer of indirection to outside-services to support further expansion. This could be a breaking change for anyone relying on the current directory structure.

…luster (default port 31111).

Added a layer of indirection to `outside-services` to support further expansion. This could be a breaking change for anyone relying on the current directory structure.
@solsson
Copy link
Contributor

solsson commented Mar 9, 2018

Sorry for the late reply. Thanks for your contribution. How about adding this service to ./avro-tools/outside-services/ to avoid the breaking? We can't assume that people read readmes - I don't, at least not until I'm stuck :) - so people might otherwise unknowingly introduce a security issue. There's still a risk, in case someone does kubectl apply -R -f ./avro-tools/, which I might have recommended for those who want to include tests.

How about patch format instead, like https://github.com/Yolean/kubernetes-kafka/blob/v3.1.0/prometheus/50-kafka-jmx-exporter-patch.yml#L2, that simply changes to NodePort for the existing service? Or maybe that's disallowed.

@clintfred
Copy link
Author

clintfred commented Mar 12, 2018

I'm not sure what the repercussions would be of changing the schema registry's port, but it seems like that would necessitate other config changes to keep things working, but that might be an option with more work.

I'm fine with moving the service into ./avro-tools/, and is in fact what I did at first. My thinking in putting it in outside-services is that's it's probably better to centralize exposing services outside the cluster as it's (seemingly) an edge case, opposed to spreading it throughout the repo. For example, where would outside access for KSQL go? My preference would be to keep kubectl apply -R -f ./avro-tools/ and kubectl apply -R -f ./ksql/ "safe" by default, and put the edge-cases in outside-services.

@solsson
Copy link
Contributor

solsson commented Mar 12, 2018

A third option is to move outside-services could move into ./kafka/. Not so good wrt recursive apply though. Maybe patch files inside outside-services is the least prone to security related surprises? I would like to keep this repo modular though, with features separated by folders. In short I'm undecided :)

@clintfred
Copy link
Author

Hey @solsson, just thought I'd check in with you on this PR. Is this something you're still interested in?

@clintfred
Copy link
Author

Closing this very old PR. If someone wants to reopen, feel free.

@clintfred clintfred closed this May 26, 2021
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