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

Update check_connection method to use empty document list index request #16

Conversation

andsel
Copy link
Contributor

@andsel andsel commented Jan 24, 2022

Release notes

Change implementation of connectivity check method to be compatible with version 8.0 of Workplace.

What does this PR do?

Change implementation of check_connection! method, replacing the call to list_all_permissions which doesn't have anymore a peer endpoint in 8.0 to an empty document list index_documents.
Fixes also the integration tests for Workplace Search version 8, switching API to retrieve the access_token

Why is it important/What is the impact to the user?

Fixes a compatibility problem with Workplace Search 8.0

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files (and/or docker env variables)
  • I have added tests that prove my fix is effective or that my feature works

Author's Checklist

How to test this PR locally

export ELASTIC_STACK_VERSION=8.x
export SNAPSHOT=true
.ci/docker-setup.sh
.ci/docker-run.sh

Related issues

@andsel andsel changed the title Switch implementation of check_connection method to use empty document list index request Update check_connection method to use empty document list index request Jan 24, 2022
@andsel andsel force-pushed the fix/workplace_check_connection_to_use_empty_index_method branch 2 times, most recently from f9bb9b9 to ac8355c Compare January 25, 2022 13:15
@andsel andsel force-pushed the fix/workplace_check_connection_to_use_empty_index_method branch from ac8355c to 615b4d2 Compare January 25, 2022 13:58
Instead of relying on the private  ws/org/sources/form_create endpoint to create a source,
this commit switches to the combination of the public ones:
- api/ws/v1/whoami to retrieve the access_token related to the basic authentication
- api/ws/v1/sources to create a custom source to be usedby the integration test
@@ -8,6 +8,7 @@ COPY --chown=logstash:logstash *.gemspec VERSION* version* /usr/share/plugins/pl
RUN cp /usr/share/logstash/logstash-core/versions-gem-copy.yml /usr/share/logstash/versions.yml
ENV PATH="${PATH}:/usr/share/logstash/vendor/jruby/bin:/usr/share/logstash/jdk/bin"
ENV LOGSTASH_SOURCE="1"
ARG ELASTIC_STACK_VERSION
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewer:
without this the ENV['ELASTIC_STACK_VERSION'] doesn't contain value

@kaisecheng kaisecheng self-requested a review January 28, 2022 11:49
Copy link

@kaisecheng kaisecheng left a comment

Choose a reason for hiding this comment

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

tested locally. fixed the issue as it tells. LGTM

"Accept" => "application/json",
"Authorization" => "Bearer #{access_token}"}
)
puts "DBG>> source_id response body: #{response.body}"

Choose a reason for hiding this comment

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

do you really want to print value here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, my bad I didn't remove it

@andsel andsel merged commit 41d51a6 into logstash-plugins:main Jan 28, 2022
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.

Fix Workplace Search test failure on 8.x due to endpoint elimination
2 participants