jakartaee / authentication

Jakarta Authentication
https://eclipse.org/ee4j/jaspic
Other
24 stars 33 forks source link

Harmonize groupId and artifactId #89

Closed keilw closed 4 years ago

keilw commented 4 years ago

While the groupId of Jakarta Security is jakarta.security.enterprise matching the top level package, this one is "jakarta.security.auth.message". And the artifactId "jakarta.security.auth.message-api". While "message" is currently the only package, with the module itself called "authentication", wouldn't it be more consistent and less confusing to just call them "jakarta.security.authentication" and "jakarta.security.authentication-api"?

arjantijms commented 4 years ago

Yes, or just after the project, jakarta.authentication and jakarta.authentication-api.

The authorization project uses exactly that in the authorization variant. See: https://github.com/eclipse-ee4j/authorization/blob/master/api/pom.xml#L30

teobais commented 4 years ago

Good point @keilw , I initiated a PR ( #97 ) which fixes that.

However, the question here (and also for authorization) is: is this indeed what we want or do we actually want to propagate the newly introduced groupId and artifactId to the package naming as well?

Example - current state

pom.xml

<groupId>jakarta.security.authentication</groupId>
<artifactId>jakarta.security.authentication-api</artifactId>

BUT

Java class

package jakarta.security.auth.message;

Example from JSR-385

pom.xml

<groupId>tech.units</groupId>
<artifactId>indriya</artifactId>

AND

Java class

package tech.units.indriya;

Am I missing something here @arjantijms ?

arjantijms commented 4 years ago

Am I missing something here @arjantijms ?

We can't just change those IDs, specifically the group ID, since the project doesn't own these.

Also, to be consistent with Jakarta Authorization, the group ID would probable need to become "jakarta.authentication" then.

keilw commented 4 years ago

@arjantijms What do you mean with the project doesn't own these? As long as it's consistent, I would not have a problem with jakarta.authentication / jakarta.authentication-api and jakarta.authorization / jakarta.authorization-api.

IMO what @thodorisbais pointed out is also worth questioning because jakarta.security.auth is totally ambigious and "auth" could mean both "authentication" and "authorization".

Last but not least, would you keep the longer namespace of jakarta.security.enterprise / jakarta.security.enterprise-api for Jakarta Security ?

I know it was there before, but it's somewhat inconsistent now.

arjantijms commented 4 years ago

@keilw

We can't just deploy to another namespace, we have to request a new group ID. I.e. "jakarta" is owned by Eclipse, and projects can request sub-ids there, e.g. "jakarta.foo". If I would now deploy to "jakarta.foo" using the credentials setup for this project, I get an error (403, denied).

For the Jakarta EE 9 release we can't really change the package, since it's a "jakarta.*" change only according to the release plan, but post Jakarta EE 9 it's of course up for the discussion. Of course "auth" and "message" are not that great. I know where Ron was coming from, but I'd like to ask the committers in this project going forward to de-emphasize the term "message" a little.

I think the ideal for the package name would simple be "jakarta.authentication" with group id "jakarta.authentication" matching the project name Jakarta Authentication.

Indeed, for security the "enterprise" makes no sense either, and I'd live to go for a "jakarta.security" group id, "jakarta.security" package matching project name Jakarta Security.

These names however are not all up to me, of course ;) The other committers need to agree and specially Eclipse and the PMC have to grant us the "jakarta.security" group id (since we have a sub of that, I think we'd be okay, but we still have to request it, I can't just take it).

keilw commented 4 years ago

@arjantijms Are these two Maven coordinates the status quo right now? jakarta.authentication / jakarta.authentication-api and jakarta.authorization / jakarta.authorization-api.

Whether the package name is changed I guess we (you) may have certain freedom and unless the "security" namespace was desired to hold certain projects together (again it might be for Architecture Council, PMC and/or Spec Committee or Steering committee to think about that, we are both in the latter two, so we could try to raise that question)

I understand, "Jakarta Security" with a slightly longer legacy even at Jakarta EE could be more problematic, so let's ignore that for now.

keilw commented 4 years ago

Actually in the master branch the parent POM is even worse:

<groupId>org.eclipse.ee4j.jaspic</groupId>
    <artifactId>jaspic-parent</artifactId>

While the API has

    <groupId>jakarta.security.auth.message</groupId>

    <artifactId>jakarta.security.auth.message-api</artifactId>

So do we keep "jakarta.security" in all the groupIds or change it to "jakarta.authentication" here and leave "jakarta.authorization" as it is?

Btw, I am unaware, you have to request a new GroupId with Sonatype, there should be a wildcard under "jakarta.*" for all users (technical or actual) who are allowed to deploy there.

teobais commented 4 years ago

@arjantijms so what is the decision here? Do we move forward with #97 ?

arjantijms commented 4 years ago

My suggestion would be to go for jakarta:authentication:jakarta:authentication-api, to be consistent with jakarta:authorization:jakarta:authorization-api.

teobais commented 4 years ago

Exactly. @keilw pointed me to the respective documentation the other day, so I guess, this is a rational decision for the time being. #97 has just been updated @arjantijms !

keilw commented 4 years ago

Sounds cool, and what about the "enterprise" in Jakarta Security, should that stay as it is for historic reasons or also change to "jakarta.security" only?

arjantijms commented 4 years ago

In Jakarta I don't think we're that big on keeping things for history sake ;) The "enterprise" makes little sense IMHO. Note that CDI and concurrency use "enterprise" too, but further up in its package and tree:

https://repo1.maven.org/maven2/jakarta/enterprise/

Wonder if we should try to make the case to turn:

<groupId>jakarta.enterprise</groupId>
<artifactId>jakarta.enterprise.cdi-api</artifactId>

into e.g.

<groupId>jakarta.cdi</groupId>
<artifactId>jakarta.cdi-api</artifactId>
keilw commented 4 years ago

Note the package name however does not contain any "cdi" so for CDI I doubt, it would work, unless they replaced the "jakarta.enterprise" with "jakarta.cdi" also on a package level for Jakarta EE 9. CDI also historically got more than one package root in the API, but as long as it does not share that package with any other spec (which would especially violate the Jigsaw modularity) it is fine, and "jakarta.enterprise" seems the more dominant one.

I trust both Authentication and Authorization are supposed to get the "security" out of the package name, because the Jakarta Maven Guidelines talk about being as consistent as possible between the groupId and the top-most Java package of the spec.

So if all the 3 should keep the parent package name "jakarta.security" then I believe it also would be inconsistent if not wrong to drop the "security" from the groupId. If in another task that should happen, then it is the question, if Jakarta Security was to also reduce the "enterprise" package level and be the only spec using "jakarta.security"?

teobais commented 4 years ago

Is this discussion relevant to this issue? I think for now we can merge #97 and link this discussion to the respective repo. Or am I missing something?

arjantijms commented 4 years ago

@thodorisbais We can't just merge 97 unfortunately, as we wouldn't be able to publish anything anymore. This project doesn't own the group id "jakarta.authentication". We have to get that and the release bot has to be given access to it.

keilw commented 4 years ago

@arjantijms Do you say it must stay so messed-up forever or do you need to get permission for "jakarta.authentication" first? And is there something we can help with submitting at Bugzilla, etc. or does it all have to be done only by project leads alone?

arjantijms commented 4 years ago

The latter of course ;)

Just ask the PMC and create the bug for the new group Id. When the ci bot has the permissions the pr can be merged and I can stage a new RC.

teobais commented 4 years ago

@arjantijms , ok, should you need any further help, please let us know.

ggam commented 4 years ago

Just ask the PMC and create the bug for the new group Id. When the ci bot has the permissions the pr can be merged and I can stage a new RC.

Did someone already started the process or can we do it now?

arjantijms commented 4 years ago

Did someone already started the process or can we do it now?

I don't think so. Let's start it by sending a mail to the PMC.

keilw commented 4 years ago

@arjantijms Are you in the PMC already or considering to run for one of the seats in Steering Committee or another one now?

teobais commented 4 years ago

Any updates here?

arjantijms commented 4 years ago

@thodorisbais Thanks for keeping on this case. The PMC question was posted and no objections were raised. So I'll go ahead and file a bug for this, stating the PMC approval.

arjantijms commented 4 years ago

Bug 🐜 created here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=562045

keilw commented 4 years ago

This has not been done here, any reason why?

arjantijms commented 4 years ago

This has not been done here, any reason why?

I think we should be good to go, and the next release can be deployed to the new GAV.

keilw commented 4 years ago

I hope you mean milestone? because Jakarta EE 9 is approaching sooner or later.

arjantijms commented 4 years ago

Yes indeed, that would be the next milestone, or if that takes too long we can do a milestone with only the new coords.

keilw commented 4 years ago

Great, as you probably know anyway just because it was mentioned in the Jakarta EE platform call, someone should also raise a PR against Glassfish once the new milestone deliverable is available.

keilw commented 4 years ago

Guess that someone can merge https://github.com/eclipse-ee4j/authentication/pull/97 here, there seems no ECA lack for @thodorisbais :-) @arjantijms or others with committer access, please look into it, I don't have here, only EE Security.

arjantijms commented 4 years ago

Job done, thanks to everyone involved :)