-
Notifications
You must be signed in to change notification settings - Fork 37
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
[BUG] Flow-framework build failed because no opensearch-security dependency #410
Comments
@peternied / @cwperks / @DarshitChanpura any idea here. We have already raised the PR based on the solution provided but still fails with the above error present in the description. |
Seems like the regular image is available in any of the mentioned locations. I confirmed by manually checking all locations except sonatype has the snapshot image: https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/plugin/opensearch-security/2.12.0.0-SNAPSHOT/ A possible solution is to compile on the fly and publish it to maven local. |
@owaiskazi19 Based on 2) in the original post, I think the security plugin can add:
to scripts/build.sh. I will raise a PR for this in the security repo. |
@zelinh @owaiskazi19 NVM, it standard practice to public to maven local during builds in jenkins - makes sense to following that trend. |
@owaiskazi19 @vibrantvarun I'm not exactly sure how zipArchive resolution works at integTest time. I'm looking at 2 examples of security-analytics and skills repo that have zipArchive dependencies on plugins other than job-scheduler:
The plugins that these repos have zipArchive dependencies on do not publish their zips to maven local for the build (same with security). Would Flow-Framework be able to use the example from security-analytics to resolve the zipArchive dependency on the security repo for the integ tests? |
@cwperks I've modified how we install our plugins into our integration test cluster, similar to how the security analytics plugin's build.gradle integTest task is configured. Testing with a non-existent version, attempting to run with security disabled (in which we do not install the security zip into our integration test cluster) still fails with the same issue :
|
@cwperks Interestingly, conditionally including the security zip archive dependency resolves this issue. For example :
Testing with a dummy version for security
Running with security enabled (dummy version value), expectedly fails :
|
@joshpalis to resolve that error you would need to publish a zip of the security plugin with the respective version to maven local. Since you are using a dummy version you would also need to modify security's build.gradle to the version you are expecting. To publish a zip to maven local you can use:
I don't currently understand how security-analytics is able to pull the required zipArchive dependencies for its integration test run |
This error isnt the issue, my use of the dummy version was to demonstrate that the build error we're seeing can be avoided if we just conditionally include the security zipArchive dependency if security is disabled (by default). So now with this change, if we attempt to assemble the the flow framework plugin and the security plugin isn't available, the build should pass. |
Seems the security analytics plugin relies on the OpenSearch docker image here in this security test workflow github action which has the security plugin. The reason we decided to use the security plugin zip archive dependency directly for the flow framework plugin is due to the flaky nature of the docker image, in several cases that I've seen, most security test github actions either fail outright or skip security integration tests since the cluster fails to start. For example, this security analytics CI run here fails due to :
|
@zelinh If Flow-Framework wants to run their integ tests at distribution build time with a zip archive of the security plugin, would they need to add it as a build dependency here? If security is added as a build dependency, does that also mean that opensearch-build would need to be extended to support it as well? According to the opensearch-build README:
How does opensearch-build provide these dependencies to the maven local repo before kicking off integ tests? |
What is the bug?
Currently I noticed the distribution bundle build for 2.12.0 failed on flow-framework plugin because it can't find opensearch-security dependency after this introduced
flow-framework/build.gradle
Line 164 in f27f2d2
More error context can be found here.
There are two approaches that we think could resolve this:
Since it says there are test time dependency, when we run build, we should exclude these test dependencies.
You could create a customized build script just like how k-NN did here. https://github.com/opensearch-project/k-NN/blob/7900dbb339ab56a56a09cf07c50381aeba084cbb/scripts/build.sh#L121 and disable any test time dependency using
-x integTest
You could copy our default build script from https://github.com/opensearch-project/opensearch-build/blob/main/scripts/default/opensearch/build.sh to your repo and made customization on that.
Ask security repo to publish to Maven Local when security plugin is built.
Currently security plugin never publishes Maven local and there isn't non-snapshot plugin available on Sonatype. https://github.com/opensearch-project/security/blob/4a9006840be3430bcb704a0e39507dc289e99a56/scripts/build.sh#L80
The text was updated successfully, but these errors were encountered: