Skip to content

Adjust installation test#100

Merged
mgreau merged 2 commits intoelastic:masterfrom
jkakavas:fix-test-security
Sep 7, 2021
Merged

Adjust installation test#100
mgreau merged 2 commits intoelastic:masterfrom
jkakavas:fix-test-security

Conversation

@jkakavas
Copy link
Copy Markdown

@jkakavas jkakavas commented Sep 5, 2021

During installation, we start elasticsearch a couple of times and
check that we can connect to it. Since elastic/elasticsearch#72300
these tests ( and installation for that matter) have been failing
since security is enabled by default and we make requests without
credentials.

This change, as a temporary measure, makes it so that we start
elasticsearch with security explicitly disabled temporarily during
installation so that we can make the necessary connection test
requests without credentials.

In a follow-up, we will adjust the Formula to take advantage of
newly added functionality, set and show the password for the
elastic user during installation (similar to what we do in
elastic/elasticsearch#75144). We can then also use this password
to make the test requests without needing to disable authN.

During installation, we start elasticsearch a couple of times and
check that we can connect to it. Since elastic/elasticsearch#72300
this tests ( and installation for that matter) has been failing
since security is enabled by default and we make requests without
credentials.

This change, as a temporary measure, makes it so that we start
elasticsearch with security explicitly disabled temporarily during
installation so that we can make the necessary connection test
requests without credentials.

In a follow-up, we will adjust the Formula to take advantage of
newly added functionality, set and show the password for the
elastic user during installation (similar to what we do in
elastic/elasticsearch#75144). We can then also use this password
to make the test requests without needing to disable authN.
@jkakavas jkakavas requested a review from mgreau September 5, 2021 19:06
Copy link
Copy Markdown

@breskeby breskeby left a comment

Choose a reason for hiding this comment

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

lgtm

@jkakavas
Copy link
Copy Markdown
Author

jkakavas commented Sep 7, 2021

@mgreau just making sure this is on your radar so that we can merge soon and unblock tests

Copy link
Copy Markdown

@mgreau mgreau left a comment

Choose a reason for hiding this comment

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

LGTM

@mgreau mgreau merged commit 56b7b0b into elastic:master Sep 7, 2021
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