search

confluentinc / schema-registry

Confluent Schema Registry for Kafka
https://docs.confluent.io/current/schema-registry/docs/index.html
Other
2.2k stars 1.11k forks source link

Use of group.id in KafkaStore Consumer ? #490

Closed simplesteph closed 7 years ago

simplesteph commented 7 years ago

Currently, the group.id is created using schema-registry-<hostname>-<first exposed port>

It seems that the code proceeds to assign() and never commits offsets, so here are the following questions:

1) What is the use of group.id ? 2) What if two schema registry share the same group.id ? 3) Should we allow users to set group.id using an environment variable?

The goal of this discussion is coming up with a good strategy to secure the schema registry.

Currently, it would have been done using: bin/kafka-acls.sh --authorizer-properties zookeeper.connect=zoo1:2181 --add --allow-principal User:schema_registry --consumer --topic _schemas --group schema-registry-<hostname>-<first exposed port>

Which works if the hostname is fixed, but sometimes schema registry will move host instances, and therefore the group.id generated by the code will change too. And because the ACLs won't be set, the schema registry will fail on launch.

gwenshap commented 7 years ago
  1. Since we are only assigning and seeking (never subscribing and never committing offsets), group.id is only used when monitoring
  2. Looks like nothing bad will happen, but if you are collecting metrics, it may look a bit confusing.
  3. I dislike environment variables. It is both non-standard in Kafka and difficult to troubleshoot (hard to know what's set in a specific environment, vs just looking at config file)

I think allowing setting the group.id as an optional and low-priority configuration is reasonable. We can keep the current generated ID as a default.

simplesteph commented 7 years ago
  1. Sorry meant java properties. I just use the confluent images to set them so they translate to env variables for me
gwenshap commented 7 years ago

makes sense :) I think we know what the change will look like now, so whenever you have a PR ready...