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

Make test cluster tasks depend on fipsResources when fips enabled #74670

Merged

Conversation

breskeby
Copy link
Contributor

The fips specific cluster specification depends on files that are declared
as output of the fips-resources task. So far there has not been an explicit
dependency of all TestClustersAware tasks (so far only for Test tasks) to this
resources task in our build and.

implicit usage of task outputs without any explicit task dependency has been
deprecated in Gradle as the build tool detects that these file are outputs of
this task and urges users to have this explicit dependency declared in a build.

RestIntegTestTask had this dependencies explicitly declared by us having defined a dependency
by declaring

tasks.withType(Test) { dependsOn 'fipsResources'}.

This has be replaced by with this PR by

tasks.withType(TestClustersAware) { dependsOn 'fipsResources'}

which also covers usages of DefaultTestClustersTask as used in the
:x-pack:plugin:eql:qa:multi-cluster-with-security project which
fails in #74664

fixes #74664

The fips specific cluster specification depends on files that are declared
as output of the fips-resources task. So far there has not been an explicit
dependency of all TestClustersAware tasks (so far only for Test tasks) to this
resources task in our build and.

implicit usage of task outputs without any explicit task dependency has been
deprecated in Gradle as the build tool detects that these file are outputs of
this task and urges users to have this  explicit dependency declared in a build.

RestIntegTestTask had this dependencies explicitly declared by us having defined a dependency
by declaring

`tasks.withType(Test) { dependsOn 'fipsResources'}`.

This has be replaced by with this PR by

`tasks.withType(TestClustersAware) { dependsOn 'fipsResources'}`

which also covers usages of `DefaultTestClustersTask` as used in the
`:x-pack:plugin:eql:qa:multi-cluster-with-security` project which
fails in elastic#74664

fixes elastic#74664
@breskeby breskeby self-assigned this Jun 29, 2021
@breskeby breskeby added :Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.14.0 v8.0.0 labels Jun 29, 2021
@breskeby breskeby requested review from ywangd and jkakavas June 29, 2021 07:28
@breskeby breskeby marked this pull request as ready for review June 29, 2021 07:28
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@breskeby
Copy link
Contributor Author

@jkakavas @ywangd I've added you as reviewers but this is more an FYI. Will merge this as soon as passed the checks to unblock the failing fips builds again

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

TIL. Thanks @breskeby 👍

@breskeby breskeby merged commit 36ae5ac into elastic:master Jun 29, 2021
@ywangd
Copy link
Member

ywangd commented Jun 29, 2021

@breskeby I got a new failure on PR #74662 after merging the master branch. Failure message is something like:

Cannot change dependencies of dependency configuration ':x-pack:plugin:async-search:qa:security:es_distro_extracted_testclusters-x-pack-plugin-async-search-qa-security-javaRestTest-1-8.0.0-SNAPSHOT' after task dependencies have been resolved

Is it related to the changes introduced here?

Build scan: https://gradle-enterprise.elastic.co/s/ptphumjq27igu

@breskeby
Copy link
Contributor Author

It shouldn't, but I'll look into this.

@breskeby
Copy link
Contributor Author

This is not a a result of this PR. I see there are a few of those failures on ci the last few days. Is this reproducible for you?

@ywangd
Copy link
Member

ywangd commented Jun 29, 2021

Thanks for looking into it. Sorry I assumed it could be due to this PR when seeing the words about dependencies.

Yes it is reproducible locally on the master branch with the followings:
./gradlew -Dtests.fips.enabled=true :example-plugins:custom-suggester:yamlRestTest
build scan: https://gradle-enterprise.elastic.co/s/x6b5p4sr7rely

@breskeby
Copy link
Contributor Author

The other issue here raised by @ywangd is addressed by this PR: #74692

@breskeby breskeby added the auto-backport Automatically create backport pull requests when merged label Jul 1, 2021
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
7.14 Commit could not be cherrypicked due to conflicts

To backport manually run:
backport --pr 74670

@breskeby breskeby removed the auto-backport Automatically create backport pull requests when merged label Jul 1, 2021
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jul 1, 2021
…astic#74670)

The fips specific cluster specification depends on files that are declared
as output of the fips-resources task. So far there has not been an explicit
dependency of all TestClustersAware tasks (so far only for Test tasks) to this
resources task in our build and.

implicit usage of task outputs without any explicit task dependency has been
deprecated in Gradle as the build tool detects that these file are outputs of
this task and urges users to have this  explicit dependency declared in a build.

RestIntegTestTask had this dependencies explicitly declared by us having defined a dependency
by declaring

`tasks.withType(Test) { dependsOn 'fipsResources'}`.

This has be replaced by with this PR by

`tasks.withType(TestClustersAware) { dependsOn 'fipsResources'}`

which also covers usages of `DefaultTestClustersTask` as used in the
`:x-pack:plugin:eql:qa:multi-cluster-with-security` project which
fails in elastic#74664

fixes elastic#74664
elasticsearchmachine pushed a commit that referenced this pull request Jul 1, 2021
…4670) (#74808)

The fips specific cluster specification depends on files that are declared
as output of the fips-resources task. So far there has not been an explicit
dependency of all TestClustersAware tasks (so far only for Test tasks) to this
resources task in our build and.

implicit usage of task outputs without any explicit task dependency has been
deprecated in Gradle as the build tool detects that these file are outputs of
this task and urges users to have this  explicit dependency declared in a build.

RestIntegTestTask had this dependencies explicitly declared by us having defined a dependency
by declaring

`tasks.withType(Test) { dependsOn 'fipsResources'}`.

This has be replaced by with this PR by

`tasks.withType(TestClustersAware) { dependsOn 'fipsResources'}`

which also covers usages of `DefaultTestClustersTask` as used in the
`:x-pack:plugin:eql:qa:multi-cluster-with-security` project which
fails in #74664

fixes #74664
breskeby added a commit that referenced this pull request Jul 5, 2021
…4670) (#74808)

The fips specific cluster specification depends on files that are declared
as output of the fips-resources task. So far there has not been an explicit
dependency of all TestClustersAware tasks (so far only for Test tasks) to this
resources task in our build and.

implicit usage of task outputs without any explicit task dependency has been
deprecated in Gradle as the build tool detects that these file are outputs of
this task and urges users to have this  explicit dependency declared in a build.

RestIntegTestTask had this dependencies explicitly declared by us having defined a dependency
by declaring

`tasks.withType(Test) { dependsOn 'fipsResources'}`.

This has be replaced by with this PR by

`tasks.withType(TestClustersAware) { dependsOn 'fipsResources'}`

which also covers usages of `DefaultTestClustersTask` as used in the
`:x-pack:plugin:eql:qa:multi-cluster-with-security` project which
fails in #74664

fixes #74664
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jul 6, 2021
This fixes a regression introduced with elastic#74670.
Vanilla test tasks (of plain type Test) must also dependOn `fipsResources`
when build is run in FIPS mode. Otherwise we see the plain test task failing with

"SHA MessageDigest not available"

Fixes elastic#74922
breskeby added a commit that referenced this pull request Jul 6, 2021
This fixes a regression introduced with #74670.
Vanilla test tasks (of plain type Test) must also dependOn `fipsResources`
when build is run in FIPS mode. Otherwise we see the plain test task failing with

"SHA MessageDigest not available"

Fixes #74922
breskeby added a commit to breskeby/elasticsearch that referenced this pull request Jul 6, 2021
This fixes a regression introduced with elastic#74670.
Vanilla test tasks (of plain type Test) must also dependOn `fipsResources`
when build is run in FIPS mode. Otherwise we see the plain test task failing with

"SHA MessageDigest not available"

Fixes elastic#74922
elasticsearchmachine pushed a commit that referenced this pull request Jul 6, 2021
This fixes a regression introduced with #74670.
Vanilla test tasks (of plain type Test) must also dependOn `fipsResources`
when build is run in FIPS mode. Otherwise we see the plain test task failing with

"SHA MessageDigest not available"

Fixes #74922
breskeby added a commit that referenced this pull request Jul 6, 2021
This fixes a regression introduced with #74670.
Vanilla test tasks (of plain type Test) must also dependOn `fipsResources`
when build is run in FIPS mode. Otherwise we see the plain test task failing with

"SHA MessageDigest not available"

Fixes #74922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.14.0 v8.0.0-alpha1
Projects
None yet
5 participants