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

Updated Kafka security configuration #2994

Merged
merged 4 commits into from
Jul 13, 2023

Conversation

kkondaka
Copy link
Collaborator

Description

Updated Kafka security configuration.

  1. Modified to allow authentication and encryption configuration to be separate so that it can extended easily later
  2. Added region and sts role arn to aws config.
  3. Modified to set MSK IAM properties in the kafka configuration when that auth mechanism is used.

Issues Resolved

[List any issues this PR will resolve]

Check List

  • [X ] New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • [ X] Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Krishna Kondaka added 2 commits July 7, 2023 05:45
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
return plainTextAuthConfig;
@AssertTrue(message = "Only one of SSL or SASL auth config must be specified")
public boolean hasSaslOrSslConfig() {
return Stream.of(sslAuthConfig, saslAuthConfig).filter(n -> n!=null).count() == 1;
Copy link
Contributor

@hshardeesi hshardeesi Jul 10, 2023

Choose a reason for hiding this comment

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

should we have a similar check on sasl_mechanism as well? I guess only one of the mechanisms should be specified.

Krishna Kondaka added 2 commits July 12, 2023 05:13
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Comment on lines +239 to +246
if (awsIamAuthConfig == AwsIamAuthConfig.ROLE) {
properties.put("sasl.jaas.config",
"software.amazon.msk.auth.iam.IAMLoginModule required " +
"awsRoleArn=\"" + awsConfig.getStsRoleArn()+
"\" awsStsRegion=\""+ awsConfig.getRegion()+"\";");
} else if (awsIamAuthConfig == AwsIamAuthConfig.DEFAULT) {
properties.put("sasl.jaas.config",
"software.amazon.msk.auth.iam.IAMLoginModule required;");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain what these properties are?

@kkondaka kkondaka merged commit f09db1a into opensearch-project:main Jul 13, 2023
22 of 24 checks passed
chenqi0805 pushed a commit that referenced this pull request Jul 19, 2023
* Add Kafka Security Configurations

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>

* Modified kafka security config. Added new fields to AwsConfig

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>

* Addressed review comments

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>

* Modified AwsConfig to have msk option that can take multiple options

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>

---------

Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Co-authored-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>
Signed-off-by: George Chen <qchea@amazon.com>
@kkondaka kkondaka deleted the kafka-security branch May 13, 2024 05:51
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