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 settings to connect to Salesforce MyDomain url #28

Merged
merged 14 commits into from
Nov 11, 2021
Merged

Add settings to connect to Salesforce MyDomain url #28

merged 14 commits into from
Nov 11, 2021

Conversation

mkreth
Copy link
Contributor

@mkreth mkreth commented Jun 24, 2021

Which use case is this pull request trying to solve

We are operating in an environment where we need to explicitly put Salesforce IPs into an allow-list to be able to connect. We are pushed to limit the number of IPs that we are requesting. For us it would hence be helpful to narrow down the number of IPs that we need to connect to. For this we would like more control over which Salesforce endpoint is used by the Salesforce input plugin for authentication and for further API requests. The latter would usually go to the instance url which is returned by the login request. The login request would always go to either login.salesforce.com or test.salesforce.com (depending on the value of the use_test_sandbox setting).

What does this pull request do

This pull request introduces a new configuration setting of the Salesforce input plugin which allows to provide the Salesforce instance url to connect to (already) for authentication and further API requests. The configuration parameter is named sfdc_instance_url and takes a string value of the form <mydomain>.my.salesforce.com, e.g. the hostname part of the instance url without protocol or path.
If a sfdc_instance_url is given, it takes precedence over the use_test_sandbox setting and controls the endpoint to which the login request is sent.
If sfdc_instance_url is not set, then the use_test_sandbox setting will control which endpoint is used.

@mkreth
Copy link
Contributor Author

mkreth commented Oct 24, 2021

Fixes #16

Copy link
Contributor

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, and thank you for adding the tests.

I have a request to avoid any ambiguity/confusion when both use_test_sandbox and sfdc_instance_url are set.

logstash-input-salesforce.gemspec Outdated Show resolved Hide resolved
lib/logstash/inputs/salesforce.rb Outdated Show resolved Hide resolved
Raise a ConfigurationError if both use_test_sandbox and sfdc_instance_url
are configured. These two options are mutually exclusive.
@mkreth mkreth requested a review from robbavey October 28, 2021 16:40
Copy link
Contributor Author

@mkreth mkreth left a comment

Choose a reason for hiding this comment

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

Requested changes have been incorporated in new version of the pull request.

Copy link
Contributor

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

Code looks great! Thank you for the contribution and your patience. I have a request on the docs, although I will pass over to @karenzone for a final review on documentation.

Would you mind incrementing the version in the gemspec file, and adding a note in the changelog explaining the change? As we are adding a new feature to the plugin, we should make this a minor update with the new version 3.1.0

Thanks again!

docs/index.asciidoc Show resolved Hide resolved
Copy link

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Left some suggestions inline for your consideration. Let me know what you think.

docs/index.asciidoc Show resolved Hide resolved
docs/index.asciidoc Outdated Show resolved Hide resolved
@mkreth mkreth requested review from robbavey and karenzone November 6, 2021 10:02
Copy link

@karenzone karenzone left a comment

Choose a reason for hiding this comment

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

Docs build cleanly and LGTM! Thanks for this contribution, and for your care in improving the documentation.

Copy link
Contributor

@robbavey robbavey left a comment

Choose a reason for hiding this comment

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

LGTM.

Thank you for your contribution and your patience on this!

@robbavey robbavey merged commit 98a24d7 into logstash-plugins:main Nov 11, 2021
@robbavey
Copy link
Contributor

This plugin has been released as version 3.1.0. Thanks again!

@mkreth mkreth deleted the feature/sfdc_instance_url branch November 11, 2021 16:53
@mkreth
Copy link
Contributor Author

mkreth commented Nov 11, 2021

Hi @karenzone, @robbavey,
Thank you for your support in getting this merged! It was a great experience working with you!

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