Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove Private restriction on write/tail #203

Merged
merged 6 commits into from
Sep 15, 2020
Merged

Conversation

jdowning
Copy link
Member

New Private & Shield Kafka addons can use kafka:topics:write and kafka:topics:tail. This PR removes that guard.

@jdowning jdowning requested a review from subakva September 14, 2020 21:16
@coveralls
Copy link

coveralls commented Sep 14, 2020

Coverage Status

Coverage decreased (-0.1%) to 89.655% when pulling 14cc73c on private_can_write_tail into 0f6a36f on main.

Copy link

@subakva subakva left a comment

Choose a reason for hiding this comment

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

I assume these commands will still fail if the customer hasn't adding their IP to the mtls list. Should we print a short message alerting them to that?

@jdowning
Copy link
Member Author

@subakva if we cannot establish a connection, produce an error indicating failure to connect:

ᐅ heroku kafka:topics:write test1 "foo" KAFKA_URL -a myapp
 ▸    Could not connect to kafka

@jdowning jdowning merged commit 733b77f into main Sep 15, 2020
@jdowning jdowning deleted the private_can_write_tail branch September 15, 2020 16:32
@subakva
Copy link

subakva commented Sep 15, 2020

@subakva if we cannot establish a connection, produce an error indicating failure to connect:

ᐅ heroku kafka:topics:write test1 "foo" KAFKA_URL -a myapp
 ▸    Could not connect to kafka

I was thinking that if it's a private service addon, write an info log: "Writing to a private kafka requires adding your IP using data:mtls:ip-rules:create. I didn't intend this to be a blocker though. :)

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.

3 participants