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

Add support for basic auth in maven schema registry plugin #907

Merged
merged 3 commits into from Oct 15, 2018
Merged

Add support for basic auth in maven schema registry plugin #907

merged 3 commits into from Oct 15, 2018

Conversation

rhysmccaig
Copy link
Contributor

@rhysmccaig rhysmccaig commented Oct 9, 2018

I can't figure out how to get this repo to build locally, but this is my best guess at a patch that will support basic auth in the maven schema registry plugin. Would be great to get thoughts from anyone with better knowledge of the repo.

Edit: Got this building and tested. Its ready to merge if a committer can take a look. See comment below.

@ghost
Copy link

ghost commented Oct 9, 2018

It looks like @rhysmccaig hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

@rhysmccaig
Copy link
Contributor Author

[clabot:check]

@ghost
Copy link

ghost commented Oct 9, 2018

@confluentinc It looks like @rhysmccaig just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@rhysmccaig
Copy link
Contributor Author

Successfully tested the following configurations after making this change:

  • No basic auth supplied
<schemaRegistryUrls>
  <param>https://schema.registry/</param>
</schemaRegistryUrls>
  • Basic auth details supplied via URL:
<schemaRegistryUrls>
  <param>https://username:password@schema.registry/</param>
</schemaRegistryUrls>
<basicAuthCredentialsSource>URL</basicAuthCredentialsSource>
  • Basic auth details supplied via userInfoConfig parameter:
<schemaRegistryUrls>
  <param>https://schema.registry/</param>
</schemaRegistryUrls>
<basicAuthCredentialsSource>USER_INFO</basicAuthCredentialsSource>
<userInfoConfig>username:password<userInfoConfig>

@mageshn mageshn requested a review from a team October 12, 2018 21:55
@mageshn
Copy link
Member

mageshn commented Oct 12, 2018

@rhysmccaig thanks a lot for getting to this. This will be super useful. Since the plugin is likely to be used from CI/CD and the credentials would also most likely be injected via CI tools, I would think that we don't have to expose the concept of basicAuthCredentialsSource. The plugin can just have properties for username and password and just user USER_INFO under the hood. It will make the configuration a lot simpler and straightforward. Thoughts?

@rhysmccaig
Copy link
Contributor Author

@mageshn I had considered that, but also wondered if perhaps it makes sense to keep the URL option in case there are multiple schema registries to push the schemas to/validate against?

I'm happy with either approach. Let me know what you think and I can push the changes if required.

@mageshn
Copy link
Member

mageshn commented Oct 12, 2018

@rhysmccaig you can only configure a plugin to go against a single SR cluster. So, I think it probably makes sense to simplify the configuration for this.

@rhysmccaig
Copy link
Contributor Author

@mageshn ahh, fair enough. no worries - ill make the changes and push shortly

@rhysmccaig
Copy link
Contributor Author

@mageshn should be good to go 👍

Copy link
Member

@mageshn mageshn left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @rhysmccaig for this contribution.

@mageshn mageshn merged commit dcd5939 into confluentinc:master Oct 15, 2018
@rhysmccaig rhysmccaig deleted the support_basic_auth branch October 15, 2018 21:15
gvishnutej pushed a commit to gvishnutej/schema-registry-plugin that referenced this pull request Mar 3, 2019
ImFlog pushed a commit to ImFlog/schema-registry-plugin that referenced this pull request Apr 11, 2019
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.

None yet

2 participants