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

Fix Kafka tutorial - change remaining topic names to mytopic #83

Closed
wants to merge 2 commits into from

Conversation

orishavit
Copy link
Contributor

@orishavit orishavit commented May 18, 2023

Description

Some parts of the tutorial used the wrong Kafka topic name (probably was changed at some point), which caused the tutorial to not work cleanly. This PR changes all references to the topic name to mytopic.

@orishavit orishavit requested a review from orishoshan May 18, 2023 08:02
@github-actions
Copy link

github-actions bot commented May 18, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@orishavit
Copy link
Contributor Author

recheck

@orishavit
Copy link
Contributor Author

I have read and understood the CLA and hereby agree to its terms by making this Pull Request Comment.

@orishavit
Copy link
Contributor Author

recheck

@orishoshan
Copy link
Contributor

@orishavit So it turns out this was intentional -- the transactions topic is supposed to be used only in the KafkaServerConfig, and elsewhere you're supposed to work with mytopic. The problem you experienced was actually caused by a bug, resolved in PR otterize/intents-operator#183.

But this tutorial is pretty confusing, and we'll be reworking it. The other text changes you made, except for those in the KafkaServerConfig - are appropriate corrections. So I suggested a change for the KafkaServerConfig back to transactions, leaving the rest as-is.

@@ -111,15 +111,15 @@ kubectl apply -f https://docs.otterize.com/code-examples/kafka-mtls/kafkaserverc
</Tabs>
</details>

Upon applying the KafkaServerConfig, an ACL will configure Kafka to allow only authenticated access to the *transactions* topic by denying all anonymous access.
Upon applying the KafkaServerConfig, an ACL will configure Kafka to allow only authenticated access to the *mytopic* topic by denying all anonymous access.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Upon applying the KafkaServerConfig, an ACL will configure Kafka to allow only authenticated access to the *mytopic* topic by denying all anonymous access.
Upon applying the KafkaServerConfig, an ACL will configure Kafka to allow only authenticated access to the *transactions* topic by denying all anonymous access.

This will be the base state, from which we will gradually roll out secure access to Kafka.

```bash
kubectl logs -n kafka statefulset/kafka | grep "Processing Acl change" | grep ANONYMOUS | tail -n 1
```
You should see the following output:
```
[2023-05-18 11:49:14,230] INFO Processing Acl change notification for ResourcePattern(resourceType=TOPIC, name=transactions, patternType=LITERAL), versionedAcls : Set(User:ANONYMOUS has DENY permission for operations: ALL from hosts: *, User:* has ALLOW permission for operations: ALL from hosts: *), zkVersion : 0 (kafka.security.authorizer.AclAuthorizer)
[2023-05-18 11:49:14,230] INFO Processing Acl change notification for ResourcePattern(resourceType=TOPIC, name=mytopic, patternType=LITERAL), versionedAcls : Set(User:ANONYMOUS has DENY permission for operations: ALL from hosts: *, User:* has ALLOW permission for operations: ALL from hosts: *), zkVersion : 0 (kafka.security.authorizer.AclAuthorizer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[2023-05-18 11:49:14,230] INFO Processing Acl change notification for ResourcePattern(resourceType=TOPIC, name=mytopic, patternType=LITERAL), versionedAcls : Set(User:ANONYMOUS has DENY permission for operations: ALL from hosts: *, User:* has ALLOW permission for operations: ALL from hosts: *), zkVersion : 0 (kafka.security.authorizer.AclAuthorizer)
[2023-05-18 11:49:14,230] INFO Processing Acl change notification for ResourcePattern(resourceType=TOPIC, name=transactions, patternType=LITERAL), versionedAcls : Set(User:ANONYMOUS has DENY permission for operations: ALL from hosts: *, User:* has ALLOW permission for operations: ALL from hosts: *), zkVersion : 0 (kafka.security.authorizer.AclAuthorizer)

@@ -12,7 +12,7 @@ spec:
keyFile: /etc/otterize-spire/key.pem
rootCAFile: /etc/otterize-spire/ca.pem
topics:
- topic: "transactions"
- topic: "mytopic"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- topic: "mytopic"
- topic: "transactions"

@orishoshan orishoshan closed this Jun 13, 2023
@orishoshan orishoshan deleted the shavit/fix-kafka-tutorial-topic branch June 13, 2023 00:03
@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2023
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.

None yet

2 participants