[BUG] zipGroup is hardcoded to "org.opensearch.plugin"

[lukas-vlcek]

Describe the bug

I am not sure if I fully understand how the pom.xml is meant to be used when OpenSearch plugins are distributed/pushed into maven repository. What concerns me is that I can see that the zipGroup variable is hardcoded to "org.opensearch.plugin" (it propagates to GroupId later).

See

OpenSearch/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java

Line 43 in 304d830

String zipGroup = "org.opensearch.plugin";

and

OpenSearch/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java

Line 53 in 304d830

String zipGroup = "org.opensearch.plugin";

This may work well for all core OpenSearch plugins but I am looking at this from the perspective of 3rd party plugin author.

If there is a 3rd party plugin having package/GroupId name "foo.bar.plugin" wouldn't this be overridden?
Are all plugin authors expected to publish plugins in "org.opensearch.plugin" GroupId regardless of the actual package name?

[saratvemulapalli]

Thanks @lukas-vlcek for opening this one. I think it was originally authored to publish opensearch-project plugin zips.

But the ask makes a lot of sense.
@prudhvigodithi @reta do you think we can set GroupId if the project doesn't set it?

[reta]

@saratvemulapalli yes, definitely, we should allow to override that

[prudhvigodithi]

Hey @lukas-vlcek thanks for opening this one, also by default the plugin behavior is to pick up the generated zips from /build/distributions/ folder and publish them to local maven (on the file system) /build/local-staging-repo , considering your example is it something ok for your plugin?

OpenSearch/buildSrc/src/main/java/org/opensearch/gradle/pluginzip/Publish.java

Lines 26 to 27 in 304d830

	public final static String LOCAL_STAGING_REPO_PATH = "/build/local-staging-repo";
	public String zipDistributionLocation = "/build/distributions/";

The background and initial Idea was to pick up the local maven published artifacts from folder/build/local-staging-repo and using CI workflows publish to end maven repo.

For more details Please check
opensearch-project/opensearch-build#716
opensearch-project/opensearch-build#1916

@dblock @bbarani @reta

[lukas-vlcek]

Perhaps we should make it clear in the documentation. It seems that some users of our plugin got the impression that they can use public mvn repo for dependency management on their side.

[prudhvigodithi]

Thanks for the suggestion I would love to improve the documentation as much as possible, just want to better understand your use case, you wanted to publish the generated zips to https://repo1.maven.org/maven2/?
@lukas-vlcek

[lukas-vlcek]

@prudhvigodithi The use case was originated by user of our plugin (see the link above), they seem to prefer to use mvn repo to manage project dependencies (which is not unusual for corporate world).
//cc @rursprung do you want to chime in ^^?

I think that originally this feature was not really designed for 3rd-party/community plugins (thus lacking the documentation).

The issue I see is that the groupId value should be really inherited from the actual package name, project groupId or at least the author should have the option to specify the groupId in the pom publishing task (the same way it is possible to specify the license or authors now). Otherwise this will create a lot of confusion (or even conflicts) when 3rd party plugins will be uploaded to [public] maven repos. I haven't been uploading artifacts to mvn repo for quite some time so I might be missing something but I assume that groupId is used as a namespace identifier that is "owned" by the author and privs/creds are required to be able to publish to it hence the groupId value is critical and should not be hardcoded.

And if we allow to define custom groupId and people will start uploading plugins to mvn repos we shell add support for that to bin/opensearch-plugin install CLI too.

Long story short we should clearly define the scope of this feature. Whether it is meant for community plugins or not. And if yes, CLI tooling will need to support it as well IMO.

HTH

[prudhvigodithi]

Hey @lukas-vlcek the group ID is fixed as org.openserach.plugin to make sure the OpenSearch plugin zips are only available in that specific group (Example: https://repo1.maven.org/maven2/org/opensearch/plugin/opensearch-ml-plugin/2.0.1.0/), this allows the plugin zips to be available at single place.

Raised a PR with some more documentation improvement.
@dblock @reta @saratvemulapalli @bbarani
Thank you

[rursprung]

thanks so much @lukas-vlcek for working on this! i didn't realise that this hadn't been tested by any 3rd party plugin yet and isn't fully working for that use-case 🙈

@prudhvigodithi The use case was originated by user of our plugin (see the link above), they seem to prefer to use mvn repo to manage project dependencies (which is not unusual for corporate world).
//cc @rursprung do you want to chime in ^^?

yes, this is exactly the use-case: we use gradle to manage everything (incl. building docker images, assembling k8s files, etc.) as it allows proper dependency management (with a ton of enterprise-y tools around it, as you can imagine). it also allows handling things like caching in a very simple manner as this is all in-built in the tooling. it's best for us if artifacts are directly published to some public maven repo (stuff like maven central is best, if it's a custom 3rd party repo we just have to vet it and then add it as a remote), otherwise we have to manually upload it internally.

@prudhvigodithi i'd strongly recommend to make the group ID configurable - the way maven is designed org.opensearch should only contain things which actually belong to the organisation owning opensearch.org (note: in some cases the domain doesn't exist, but it's the logical reasoning which counts: if ACME Inc. publishes something to com.acme this is ok even if they don't have a website under that name; however, they shouldn't publish anything to org.opensearch). so 3rd party plugins should be published under their own group ID (e.g. org.example.opensearch.awesome-plugin).
this also allows e.g. companies building plugins internally (we aren't doing that right now, so this is hypothetical from my point of view but might be very real for others) to use your gradle plugin to take care of the maven part and then publish it to an internal maven repo - with the proper naming (companies generally want to use their standard group ID for internal stuff and not other names).

[reta]

@rursprung I second your concerns, I believe using org.opensearch.plugin as group makes a lot of sense for official OpenSearch plugins but cuts off the community plugins right away. We should let others to publish ZIP under their own group identifiers, sharing the publish rights under org.opensearch.plugin is very risky (at least).

How could it work with CLI? We do have community plugins listed [1] (among others), the Apache Maven coordinates for such known plugins could be baked in (or optionally, provided via configuration settings). At worst, the CLI could be fed with Apache Maven artifact URL, the plugin will be installed as well.

@prudhvigodithi @dblock thoughts?

[1] https://opensearch.org/community_projects/

[dblock]

+1 on making the hard-coded group configurable, and we can see what the next problem is after that?

[lukas-vlcek]

If no one is already working on this I am happy to take it.

[prudhvigodithi]

The default should be as org.opensearch.plugin, unless there is an override of groupID is passed.
(zipGroup)

[lukas-vlcek]

FYI: I did not start on this yet, I will have a change to start on it in one week (after vacation). If that is too long feel free to take it in the meantime.
Edit: On it now...

[lukas-vlcek]

@prudhvigodithi @dblock
Please take a look at #4156

In general there is no hard-coded value to default to. The groupId should be always taken from project.group (see resources mentioned here) and then it is the responsibility of the developer to configure the gradle project appropriately.

If the project.group value is not provided then the build will fail (see relevant test that I created for this case https://github.com/opensearch-project/OpenSearch/pull/4156/files#diff-ecd94c54475d67ab08078a5cdb6e381c01094427bad63b70c4e67c7978b3d895R58-R70) in which case the failure is a product of the maven-publish plugin (or its dependencies).