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

Excluded integTest from build script to handle the security plugin dependency #418

Merged
merged 4 commits into from
Jan 20, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ repositories {

configurations {
zipArchive
secureIntegTestPluginArchive
}

dependencies {
Expand All @@ -162,7 +163,7 @@ dependencies {

// ZipArchive dependencies used for integration tests
zipArchive group: 'org.opensearch.plugin', name:'opensearch-ml-plugin', version: "${opensearch_build}"
zipArchive group: 'org.opensearch.plugin', name:'opensearch-security', version: "${opensearch_build}"
secureIntegTestPluginArchive group: 'org.opensearch.plugin', name:'opensearch-security', version: "${opensearch_build}"

configurations.all {
resolutionStrategy {
Expand All @@ -183,7 +184,7 @@ ext{
configureSecurityPlugin = { OpenSearchCluster cluster ->

// Retrieve Security Plugin Zip from zipArchive
configurations.zipArchive.asFileTree.each {
configurations.secureIntegTestPluginArchive.asFileTree.each {
if(it.name.contains("opensearch-security")) {
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
cluster.plugin(provider(new Callable<RegularFile>(){
@Override
Expand Down Expand Up @@ -347,7 +348,6 @@ testClusters.integTest {

// Installs all registered zipArchive dependencies on integTest cluster nodes except security
configurations.zipArchive.asFileTree.each {
if(!it.name.contains("opensearch-security")) {
Copy link
Member

Choose a reason for hiding this comment

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

Hey @owaiskazi19 instead of new secureIntegTestPluginArchive configuration, I assume just changing the if statement to if(it.name.contains("opensearch-security")) should work. Can you please check?

Copy link
Member Author

Choose a reason for hiding this comment

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

@prudhvigodithi here we have to install all the plugins except security plugin. zipArchive would have security plugin in it.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, +1 to @owaiskazi19 . Initially we kept it how @prudhvigodithi you mentioned above. But the problem, here we just need security plugin to run integ tests, therefore zip archive tries to find it at the compile time also. Therefore, we need to change the tag to securePluginTestArchive

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @vibrantvarun and @owaiskazi19 I'm afraid introducing new configuration secureIntegTestPluginArchive might never be called. Also I noticed the logic is called in ext (extra properties) where as I see similar code in integTest, If not needed we can completely remove the in ext.

testClusters.integTest {
    testDistribution = "ARCHIVE"

    // Optionally install security
    if (System.getProperty("security.enabled") != null && System.getProperty("security.enabled") == "true") {
        configureSecurityPlugin(testClusters.integTest)
    }

    // Installs all registered zipArchive dependencies on integTest cluster nodes except security
    configurations.zipArchive.asFileTree.each {
            plugin(provider(new Callable<RegularFile>(){
                    @Override
                    RegularFile call() throws Exception {
                        return new RegularFile() {
                            @Override
                            File getAsFile() {
                                return it
                            }
                        }
                    }
                })
            )

    }

plugin(provider(new Callable<RegularFile>(){
@Override
RegularFile call() throws Exception {
Expand All @@ -360,7 +360,7 @@ testClusters.integTest {
}
})
)
}

}

// Install Flow Framwork Plugin on integTest cluster nodes
Expand Down
Loading