Added opensearch.pluginzip plugin by prudhvigodithi · Pull Request #31 · opensearch-project/opensearch-plugin-template-java

prudhvigodithi · 2022-05-03T01:45:46Z

Signed-off-by: pgodithi pgodithi@amazon.com

Description

Add opensearch.pluginzip custom gradle plugin to the template.

Issues Resolved

opensearch-project/opensearch-build#1916
opensearch-project/opensearch-plugins#144

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: pgodithi <pgodithi@amazon.com>

prudhvigodithi · 2022-05-03T01:56:40Z

Hey @reta and @dblock, I have included the custom plugin opensearch.pluginzip to the template, please check.

reta · 2022-05-03T12:12:18Z

.github/workflows/CI.yml

    - name: Build and Run Tests
      run: |
          ./gradlew build
+          ./gradlew help --task publishPluginZipPublicationToZipStagingRepository


Looks like we just need ./gradlew publishPluginZipPublicationToZipStagingRepository?

@reta, I wanted to check opinions here, yes even this ./gradlew publishPluginZipPublicationToZipStagingRepository can be added, but do we need to, I see there are multiple custom plugins already added to the template, but we dont run a plugin specific task, just ./gradlew build exists, do we even need to add publishPluginZipPublicationToZipStagingRepository to test the CI?

Ah I see, I think for the sake of checking that publishing works, we could publish to local repo: publishPluginZipPublicationToZipLocalRepository

Yes we can add ./gradlew publishPluginZipPublicationToZipStagingRepository, also moving forward its better we add some validation by running the custom plugin tasks, so that way we can some evidence that the custom plugins task actually work, I will add ./gradlew publishPluginZipPublicationToZipStagingRepository to the CI.yaml.

reta · 2022-05-03T12:15:07Z

settings.gradle

 */

 rootProject.name = 'plugin-template'
+startParameter.excludedTaskNames=["validatePluginZipPom"]


Sorry, where this setting comes from?

@reta This is part of the plugin setup, usually all plugins managed build scripts or default build scripts run assemble task and publish tasks (specific to repo), example job-scheduler, but not direct build task, if called we need to exclude this validatePluginZipPom as general build task will include this, if this task called it will fail with POM validation with the error

> Task :validatePluginZipPom FAILED Execution optimizations have been disabled for task ':validatePluginZipPom' to ensure correctness due to the following reasons: - Gradle detected a problem with the following location: '/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom'. Reason: Task ':validatePluginZipPom' uses this output of task ':generatePomFileForNebulaPublication' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4.2/userguide/validation_problems.html#implicit_dependency for more details about this problem. name is missing in [/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom] description is missing in [/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom] licenses is empty in [/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom] developers is empty in [/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom]

Ideally we dont need to run this task as we define the POM values during actual publish.

Just to add I have also added this point to the plugin usage document (PR under review)

This still looks wrong to me, shouldn't we be fixing the POM by adding these fields? Open a new issue?

It was fixed after adding

allprojects { project.ext.licenseName = 'The Apache Software License, Version 2.0' project.ext.licenseUrl = 'http://www.apache.org/licenses/LICENSE-2.0.txt' publishing { publications { all { pom.withXml { XmlProvider xml -> Node node = xml.asNode() node.appendNode('inceptionYear', '2022') node.appendNode('name', 'rename') node.appendNode('description', 'Custom plugin') Node license = node.appendNode('licenses').appendNode('license') license.appendNode('name', project.licenseName) license.appendNode('url', project.licenseUrl) Node developer = node.appendNode('developers').appendNode('developer') developer.appendNode('name', 'OpenSearch') developer.appendNode('url', 'https://github.com/opensearch-project/opensearch-plugin-template-java') } } } } }

Then we dont need to exclude, I would say we can add the above template to the build.gradle and it's up-to the plugin team, if they want to exclude the the task validatePluginZipPom or add the POM fields in right way, I can update the info in the document as well, thoughts? @dblock

excluding is better, rather than forcing user to add these POM fields as then it allow user to run ./gradlew build, without any restrictions, @dblock @reta @bbarani @peterzhuamazon

I don't think I understand. Plugins should all look the same and have all the right/same info in the POM. Why wouldn't this be in all plugins?

some plugins do not have all of these settings @dblock, but in our case to publish zips we dont need this task validatePluginZipPom.
This is coming from PomValidationTask (org.opensearch.gradle.precommit.PomValidationTask)

Signed-off-by: pgodithi <pgodithi@amazon.com>

.github/workflows/CI.yml

Signed-off-by: pgodithi <pgodithi@amazon.com>

reta

@prudhvigodithi so I pulled the pull request locally and it seems like more work is needed:

> Task :publishPluginZipPublicationToZipStagingRepository
Execution optimizations have been disabled for task ':publishPluginZipPublicationToZipStagingRepository' to ensure correctness due to the following reasons:
  - Gradle detected a problem with the following location: 'opensearch-plugin-template-java/build/distributions/rename-unspecified.pom'. Reason: Task ':publishPluginZipPublicationToZipStagingRepository' uses this output of task ':generatePomFileForNebulaPublication' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4.2/userguide/validation_problems.html#implicit_dependency for more details about this problem.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

Those warnings should not be there, we need to find the way to properly address POM file exclusion or/and generation

prudhvigodithi · 2022-05-04T14:35:42Z

Those warnings should not be there, we need to find the way to properly address POM file exclusion or/and generation

Hey @reta, thanks for testing,
In this case I can see several warning with ./gradlew build that also includes validatePluginZipPom.

 ./gradlew build
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 7.4.2
  OS Info               : Linux 5.4.186-113.361.amzn2int.x86_64 (amd64)
  JDK Version           : 17 (Eclipse Temurin JDK)
  JAVA_HOME             : /opt/java/openjdk-17
  Random Testing Seed   : 6130E2759B36F54C
  In FIPS 140 mode      : false
=======================================

> Task :validatePluginZipPom
Execution optimizations have been disabled for task ':validatePluginZipPom' to ensure correctness due to the following reasons:
  - Gradle detected a problem with the following location: '/usr/share/opensearch/git/opensearch-plugin-template-java/build/distributions/rename-unspecified.pom'. Reason: Task ':validatePluginZipPom' uses this output of task ':generatePomFileForNebulaPublication' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed. Please refer to https://docs.gradle.org/7.4.2/userguide/validation_problems.html#implicit_dependency for more details about this problem.

> Task :integTest
OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.BootstrapForTesting (file:/usr/share/opensearch/.gradle/caches/modules-2/files-2.1/org.opensearch.test/framework/2.0.0-rc1-SNAPSHOT/7bca874662f27c649cbba03d409417f3b7c22c4b/framework-2.0.0-rc1-SNAPSHOT.jar)
WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/usr/share/opensearch/.gradle/wrapper/dists/gradle-7.4.2-bin/48ivgl02cpt2ed3fh9dbalvx8/gradle-7.4.2/lib/plugins/gradle-testing-base-7.4.2.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

> Task :test
OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.BootstrapForTesting (file:/usr/share/opensearch/.gradle/caches/modules-2/files-2.1/org.opensearch.test/framework/2.0.0-rc1-SNAPSHOT/7bca874662f27c649cbba03d409417f3b7c22c4b/framework-2.0.0-rc1-SNAPSHOT.jar)
WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/usr/share/opensearch/.gradle/wrapper/dists/gradle-7.4.2-bin/48ivgl02cpt2ed3fh9dbalvx8/gradle-7.4.2/lib/plugins/gradle-testing-base-7.4.2.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

> Task :yamlRestTest
OpenJDK 64-Bit Server VM warning: Ignoring option --illegal-access=warn; support was removed in 17.0
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.opensearch.bootstrap.BootstrapForTesting (file:/usr/share/opensearch/.gradle/caches/modules-2/files-2.1/org.opensearch.test/framework/2.0.0-rc1-SNAPSHOT/7bca874662f27c649cbba03d409417f3b7c22c4b/framework-2.0.0-rc1-SNAPSHOT.jar)
WARNING: Please consider reporting this to the maintainers of org.opensearch.bootstrap.BootstrapForTesting
WARNING: System::setSecurityManager will be removed in a future release
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/usr/share/opensearch/.gradle/wrapper/dists/gradle-7.4.2-bin/48ivgl02cpt2ed3fh9dbalvx8/gradle-7.4.2/lib/plugins/gradle-testing-base-7.4.2.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.4.2/userguide/command_line_interface.html#sec:command_line_warnings

Execution optimizations have been disabled for 1 invalid unit(s) of work during this build to ensure correctness.
Please consult deprecation warnings for more details.

however as we dont need this task validatePluginZipPom, once I disable it I dont see any warnings

./gradlew publishPluginZipPublicationToZipStagingRepository
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 7.4.2
  OS Info               : Linux 5.4.186-113.361.amzn2int.x86_64 (amd64)
  JDK Version           : 17 (Eclipse Temurin JDK)
  JAVA_HOME             : /opt/java/openjdk-17
  Random Testing Seed   : 3D31CDE948265F62
  In FIPS 140 mode      : false
=======================================

BUILD SUCCESSFUL in 583ms
8 actionable tasks: 2 executed, 6 up-to-date

Also I can see ls -ltr build/local-staging-repo/org/opensearch/plugin/plugin-template/, the zips being published to local maven repo

I have also tested this with job-scheduler and AD plugin, does not throw any errors, thoughts @reta ?
Should we consider still fixing this task validatePluginZipPom?

prudhvigodithi · 2022-05-04T15:06:00Z

Dont want to go with hacks, but looks like validatePluginZipPom is coming from

./gradlew help --task  validatePluginZipPom
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 7.4.2
  OS Info               : Linux 5.4.186-113.361.amzn2int.x86_64 (amd64)
  JDK Version           : 17 (Eclipse Temurin JDK)
  JAVA_HOME             : /opt/java/openjdk-17
  Random Testing Seed   : 6AD3909B8FF392E7
  In FIPS 140 mode      : false
=======================================

> Task :help
Detailed task information for validatePluginZipPom

Path
     :validatePluginZipPom

Type
     PomValidationTask (org.opensearch.gradle.precommit.PomValidationTask)

Description
     -

Group
     -

We might to add this way https://github.com/opensearch-project/OpenSearch/blob/896b97e54df4e192a15994a1076a9d1d04312f74/modules/lang-painless/build.gradle#L80-L83
so that that implicit dependency warning will go away
@dblock @reta

reta · 2022-05-04T16:51:01Z

We might to add this way https://github.com/opensearch-project/OpenSearch/blob/896b97e54df4e192a15994a1076a9d1d04312f74/modules/lang-painless/build.gradle#L80-L83
so that that implicit dependency warning will go away
@dblock @reta

@prudhvigodithi looking, once sec

prudhvigodithi · 2022-05-04T16:53:15Z

We might to add this way https://github.com/opensearch-project/OpenSearch/blob/896b97e54df4e192a15994a1076a9d1d04312f74/modules/lang-painless/build.gradle#L80-L83
so that that implicit dependency warning will go away
@dblock @reta

@prudhvigodithi looking, once sec

I have tried this way @reta, still get that warning :)
I'm also looking as I can see existing plugin setting is expecting this, not from this new change.

Signed-off-by: pgodithi <pgodithi@amazon.com>

dblock

This LGTM!

dblock · 2022-05-06T22:19:23Z

@reta re-review?

prudhvigodithi · 2022-05-06T22:25:56Z

Hey @dblock, since the fix was pushed yesterday, the plugin exists but I dont think the fixed code will get into 2.0.0-rc1-SNAPSHOT as this is already shipped, this wont break but we should still expect the warning message on console, in the next OpenSearch version release these warnings should be fixed as well.

reta · 2022-05-07T02:14:05Z

@reta re-review?

@dblock LGTM for now, but I will try to revisit the POM publication part [1], it feels like better option exists

[1] https://github.com/opensearch-project/opensearch-plugin-template-java/pull/31/files#diff-49a96e7eea8a94af862798a45174e6ac43eb4f8b4bd40759b5da63ba31ec3ef7R31

reta · 2022-05-09T14:47:10Z

@dblock @prudhvigodithi opened this one opensearch-project/OpenSearch#3252 so we could just do:

allprojects {
    project.ext.licenseName = 'The Apache Software License, Version 2.0'
    project.ext.licenseUrl = 'http://www.apache.org/licenses/LICENSE-2.0.txt'
    project.ext.developerName = 'OpenSearch'
    project.ext.developerUrl = 'https://github.com/opensearch-project/OpenSearch'
}

prudhvigodithi · 2022-05-09T23:34:10Z

Hey @reta, thanks for the PR, but I see some existing plugins already use the syntax as of which I have created the PR example for job-scheduler, this is used for the jar's to get those POM fields, now with PR looks like the plugins team should add allProjects settings with two types one with general way (like how I have added in PR) for the jar files and the way how you just added for zip's. Does this not add some some duplicate code blocks? if so we might need to add the fix for existing Publish plugin as well.
@dblock @bbarani

reta · 2022-05-10T00:02:06Z

Hey @reta, thanks for the PR, but I see some existing plugins already use the syntax as of which I have created the PR example for job-scheduler, this is used for the jar's to get those POM fields, now with PR looks like the plugins team should add allProjects settings with two types one with general way (like how I have added in PR) for the jar files and the way how you just added for zip's. Does this not add some some duplicate code blocks? if so we might need to add the fix for existing Publish plugin as well. @dblock @bbarani

I think we are dealing with different publications here: for JAR and for ZIP, which essentially generate different POM files (at least, it should), but thanks for the hit, I will make sure we could use this model as well.

prudhvigodithi · 2022-05-10T00:25:29Z

I think we are dealing with different publications here: for JAR and for ZIP, which essentially generate different POM files (at least, it should), but thanks for the hit, I will make sure we could use this model as well.

I think @reta we can go with this PR for now and add this fix to both JAR and ZIP's as next new enhancement, thoughts ? @reta
Right now anyhow plugin teams are passing how I have added to this PR, that should add for both JAR and ZIP's.

reta · 2022-05-10T00:31:48Z

I think we are dealing with different publications here: for JAR and for ZIP, which essentially generate different POM files (at least, it should), but thanks for the hit, I will make sure we could use this model as well.

I think @reta we can go with this PR for now and add this fix to both JAR and ZIP's as next new enhancement, thoughts ? @reta Right now anyhow plugin teams are passing how I have added to this PR, that should add for both JAR and ZIP's.

I am fine with merging it as-is, thanks @prudhvigodithi

Signed-off-by: pgodithi <pgodithi@amazon.com>

dblock

I'd be happy to merge on green against any version of OpenSearch.

reta · 2022-05-10T19:41:36Z

build.gradle

 buildscript {
    ext {
-        opensearch_version = "2.0.0-SNAPSHOT"
+        opensearch_version = "3.0.0-SNAPSHOT"


@prudhvigodithi this is right change in general, but for plugins we should target 2.0.0 at most - we expect most of the development happening here.

Signed-off-by: pgodithi <pgodithi@amazon.com>

prudhvigodithi · 2022-05-10T19:50:27Z

Hey @dblock and @reta we have a --component build now available PR opensearch-project/opensearch-build#2100, the OpenSearch was build with 2.0.0 manifest and I can see the latetst timestamp in nexus https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/opensearch/2.0.0-SNAPSHOT/
All checks pass :)

prudhvigodithi · 2022-05-10T20:53:31Z

Hey @reta, @dblock, @bbarani if you have merge access can you please merge this PR

prudhvigodithi · 2022-05-10T21:06:07Z

Thanks @reta

Addec opensearch.pluginzip plugin

046bc8c

Signed-off-by: pgodithi <pgodithi@amazon.com>

prudhvigodithi changed the title ~~Addec opensearch.pluginzip plugin~~ Added opensearch.pluginzip plugin May 3, 2022

Added 2.0.0-rc1-SNAPSHOT

7e5f4ac

Signed-off-by: pgodithi <pgodithi@amazon.com>

prudhvigodithi mentioned this pull request May 3, 2022

[Meta] Mechanism to publish the plugin zips to maven repo. opensearch-project/opensearch-build#1916

Closed

38 tasks

reta reviewed May 3, 2022

View reviewed changes

Added publishPluginZipPublicationToZipStagingRepository

c0f6192

Signed-off-by: pgodithi <pgodithi@amazon.com>

reta approved these changes May 3, 2022

View reviewed changes

dblock reviewed May 3, 2022

View reviewed changes

.github/workflows/CI.yml Outdated Show resolved Hide resolved

fixed NIT's

ddeb3db

Signed-off-by: pgodithi <pgodithi@amazon.com>

reta requested changes May 4, 2022

View reviewed changes

prudhvigodithi requested a review from reta May 4, 2022 16:42

reta mentioned this pull request May 4, 2022

Fixing publishZip publication by adding the dependency to generatePomFileForNebulaPublication opensearch-project/OpenSearch#3183

Closed

5 tasks

prudhvigodithi mentioned this pull request May 4, 2022

Gradle plugin opensearch.pluginzip Add implicit dependency. opensearch-project/OpenSearch#3189

Merged

5 tasks

Add implicit dependency

dcb899d

Signed-off-by: pgodithi <pgodithi@amazon.com>

dblock approved these changes May 6, 2022

View reviewed changes

reta approved these changes May 7, 2022

View reviewed changes

reta mentioned this pull request May 9, 2022

Allow to configure POM for ZIP publication opensearch-project/OpenSearch#3252

Merged

5 tasks

prudhvigodithi mentioned this pull request May 9, 2022

Remove DCO workflow as the app is taking care of the task opensearch-project/opensearch-build#2105

Merged

prudhvigodithi added 2 commits May 10, 2022 12:37

added POM

4aa4a58

Signed-off-by: pgodithi <pgodithi@amazon.com>

Added 3.0.0-SNAPSHOT

7674ea8

Signed-off-by: pgodithi <pgodithi@amazon.com>

prudhvigodithi mentioned this pull request May 10, 2022

Updated readme: Pass custom POM extensions to zip publication: opensearch-project/opensearch-plugins#149

Merged

dblock approved these changes May 10, 2022

View reviewed changes

reta reviewed May 10, 2022

View reviewed changes

Updated OS version

d8a7ee5

Signed-off-by: pgodithi <pgodithi@amazon.com>

reta merged commit 12d756e into opensearch-project:main May 10, 2022

lukas-vlcek mentioned this pull request Jun 22, 2022

[BUG] Custom POM configuration for ZIP publication produces duplicit tags (url, scm) opensearch-project/OpenSearch#3649

Closed

rursprung mentioned this pull request Sep 11, 2023

[BUG] CI doesn't fail anymore if gradlew build fails #60

Closed

Conversation

prudhvigodithi commented May 3, 2022

Description

Issues Resolved

Uh oh!

prudhvigodithi commented May 3, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prudhvigodithi May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dblock May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prudhvigodithi May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reta left a comment

Choose a reason for hiding this comment

Uh oh!

prudhvigodithi commented May 4, 2022

Uh oh!

prudhvigodithi commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reta commented May 4, 2022

Uh oh!

prudhvigodithi commented May 4, 2022

Uh oh!

dblock left a comment

Choose a reason for hiding this comment

Uh oh!

dblock commented May 6, 2022

Uh oh!

prudhvigodithi commented May 6, 2022

Uh oh!

reta commented May 7, 2022

Uh oh!

reta commented May 9, 2022

Uh oh!

prudhvigodithi commented May 9, 2022

Uh oh!

reta commented May 10, 2022

Uh oh!

prudhvigodithi commented May 10, 2022

Uh oh!

reta commented May 10, 2022

Uh oh!

dblock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prudhvigodithi May 3, 2022 •

edited

Loading

dblock May 3, 2022 •

edited

Loading

dblock May 4, 2022 •

edited

Loading

prudhvigodithi May 4, 2022 •

edited

Loading

prudhvigodithi commented May 4, 2022 •

edited

Loading