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

[bitnami/schema-registry] Relax ssl options verifications in schema registry #74972

Conversation

michalmisiewicz
Copy link
Contributor

@michalmisiewicz michalmisiewicz commented Nov 18, 2024

Description of the change

The Schema Registry libschemaregistry.sh script enforces too strict validation of SSL environment variables, which prevents integration with Google Cloud Managed Service for Apache Kafka. This service requires SASL_SSL authentication without the use of a keystore or truststore. Details of the required configurations can be found here.

In this PR, I relaxed the SSL validation rules to enable seamless integration with Managed Kafka.

I removed also the verification for SCHEMA_REGISTRY_KAFKA_SASL_USERS and SCHEMA_REGISTRY_KAFKA_SASL_PASSWORDS, as these settings are not required for all sasl.mechanism configurations. Example of configuration for sasl.mechanism= OAUTHBEARER can be found here.

Benefits

Schema registry image can connect to Google Cloud Managed Service for Apache Kafka.

Applicable issues

Additional information

I tested the update with Google Cloud Managed Service for Apache Kafka and successfully established a secure connection with Kafka.

@github-actions github-actions bot added schema-registry triage Triage is needed labels Nov 18, 2024
@github-actions github-actions bot requested a review from javsalgar November 18, 2024 14:26
@michalmisiewicz michalmisiewicz force-pushed the feat/schema-registry/relax-ssl-options-verification branch from f6e345f to 356cca6 Compare November 18, 2024 14:30
@michalmisiewicz michalmisiewicz changed the title Relax ssl options verifications in schema registry [bitnami/schema-registry] Relax ssl options verifications in schema registry Nov 18, 2024
@michalmisiewicz michalmisiewicz force-pushed the feat/schema-registry/relax-ssl-options-verification branch from 356cca6 to a7651cc Compare November 18, 2024 14:53
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Nov 18, 2024
@github-actions github-actions bot removed the triage Triage is needed label Nov 18, 2024
@github-actions github-actions bot removed the request for review from javsalgar November 18, 2024 17:22
@github-actions github-actions bot requested a review from juan131 November 18, 2024 17:22
Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

LGTM! Waiting for CI to be successful

Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

After rethinking about this, I think we could preserve validations but using warnings instead of errors given they're applicable for most common use cases. Full example:

            if [[ "$brokers_auth_protocol" =~ SSL ]]; then
                if [[ ! -f ${SCHEMA_REGISTRY_CERTS_DIR}/schema-registry.keystore.jks ]] || [[ ! -f ${SCHEMA_REGISTRY_CERTS_DIR}/schema-registry.truststore.jks ]]; then
                    warn "In order to configure the TLS encryption for communication with Kafka brokers, most auth protocols require mounting your schema-registry.keystore.jks and schema-registry.truststore.jks certificates to the ${SCHEMA_REGISTRY_CERTS_DIR} directory."
                fi
            fi
            if [[ "$brokers_auth_protocol" =~ SASL ]]; then
                if [[ -z "$SCHEMA_REGISTRY_KAFKA_SASL_USERS" ]] || [[ -z "$SCHEMA_REGISTRY_KAFKA_SASL_PASSWORDS" ]]; then
                    warn "In order to configure SASL authentication for Kafka, most auth protocols require providing the SASL credentials. Set the environment variables SCHEMA_REGISTRY_KAFKA_SASL_USERS and SCHEMA_REGISTRY_KAFKA_SASL_PASSWORDS if your auth protocol requires it."
                fi
            fi

@michalmisiewicz michalmisiewicz force-pushed the feat/schema-registry/relax-ssl-options-verification branch from a7651cc to 578e2de Compare November 20, 2024 12:22
@michalmisiewicz
Copy link
Contributor Author

@juan131 I've just updated the code

@michalmisiewicz
Copy link
Contributor Author

I’ve just tested the latest version with Managed Kafka, and it works perfectly

Copy link
Contributor

@juan131 juan131 left a comment

Choose a reason for hiding this comment

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

Thanks so much for applying the suggestions!

@juan131 juan131 enabled auto-merge (squash) November 20, 2024 13:04
@juan131 juan131 merged commit 3484146 into bitnami:main Nov 20, 2024
12 checks passed
@juan131
Copy link
Contributor

juan131 commented Nov 20, 2024

Triggering a new container release, it should be available in the next few hours

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
schema-registry solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Too strict validation of environment variables in the schema registry
4 participants