Skip to content
Closed
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.hadoop.security.authentication.server.HttpConstants;
import org.apache.hadoop.security.authentication.util.AuthToken;
import org.apache.hadoop.security.authentication.util.KerberosUtil;
import org.apache.hadoop.util.subject.SubjectAdapter;
import org.ietf.jgss.GSSContext;
import org.ietf.jgss.GSSManager;
import org.ietf.jgss.GSSName;
Expand All @@ -35,8 +36,6 @@
import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.HashMap;
Expand Down Expand Up @@ -300,8 +299,7 @@ private boolean isNegotiate(HttpURLConnection conn) throws IOException {
private void doSpnegoSequence(final AuthenticatedURL.Token token)
throws IOException, AuthenticationException {
try {
AccessControlContext context = AccessController.getContext();
Subject subject = Subject.getSubject(context);
Subject subject = SubjectAdapter.getSubject();
if (subject == null
|| (!KerberosUtil.hasKerberosKeyTab(subject)
&& !KerberosUtil.hasKerberosTicket(subject))) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.util.subject;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add a package-info.java in this new sub-directory and annotate it as private and unstable?

Example: https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/concurrent/package-info.java


import javax.security.auth.Subject;
import java.security.AccessControlContext;
import java.security.AccessController;

/**
* Calls Subject methods directly, as this class should not be classloaded on Java 18 and above
*/
class ClassicSubjectAdapter implements HiddenSubjectAdapter {

ClassicSubjectAdapter() {}

@Override
public Subject getSubject() {
final AccessControlContext context = AccessController.getContext();
return Subject.getSubject(context);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.util.subject;

import javax.security.auth.Subject;

public interface HiddenSubjectAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like the name, maybe rename to JDKSpecificSubjectAdapter or similar ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be package-private instead of public?

Subject getSubject();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.util.subject;

import javax.security.auth.Subject;

/**
* javax.security.auth.Subject.getSubject is deprecated for removal.
* The replacement API exists only in Java 18 and above.
* This class helps use the newer API if available, without raising the language level.
*/
public class SubjectAdapter {
private static final HiddenSubjectAdapter instance;
static {
int version = 0;
try {
version = Integer.parseInt(System.getProperty("java.specification.version"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Shell.isJavaVersionAtLeast(18)

Copy link
Author

Choose a reason for hiding this comment

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

My comment about dependencies applies to Shell as well. Let me know how you'd like to proceed with this one as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO testing trying to load the new classes and failing over to the old classes if they cannot be found would be more robust.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO testing trying to load the new classes and failing over to the old classes if they cannot be found would be more robust.

interesting thought.

how about

  • we avoid Subject
  • pull the string on L33 to a constant

Copy link
Contributor

Choose a reason for hiding this comment

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

@stoty interesting thought about attempting and fallbacks

We'd try the java18+ first & fall back to java <= 17 otherwise? or do it in the opposite direction, at least for now?

} catch (Throwable ignored) {}
if (version >= 18) {
instance = new SubjectAdapterJava18AndAbove();
} else {
instance = new ClassicSubjectAdapter();
}
}

private SubjectAdapter() {}

public static Subject getSubject() {
return instance.getSubject();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.util.subject;

import javax.security.auth.Subject;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;

/**
* Indirectly calls Subject.current(), which exists in Java 18 and above only
*/
class SubjectAdapterJava18AndAbove implements HiddenSubjectAdapter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use our new DynMethods classes, (which we copied from parquet and which is also found in iceberg). It fails better and as it is so common it's good to get familiar with it

Copy link
Author

Choose a reason for hiding this comment

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

@steveloughran my comment from here still applies. #7081 (comment)

private final Method currentMethod;

SubjectAdapterJava18AndAbove() {
try {
currentMethod = Subject.class.getMethod("current");
} catch (NoSuchMethodException e) {
throw new RuntimeException("Unable to find javax.security.auth.Subject.current() method", e);
}
}

@Override
public Subject getSubject() {
try {
return (Subject) currentMethod.invoke(null);
} catch (IllegalAccessException | InvocationTargetException e) {
// we would return null, but null has meaning here
throw new RuntimeException("Unable to call Subject.current()", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.apache.hadoop.util.subject;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;

class TestSubjectAdapter {

@Test
void getSubject() {
// how getSubject operates depends on the JVM calling it.
// asserting that it does not throw is a valid test, especially on Java 18 and above
// prior to Java 18, this method is just a simple wrapper
assertDoesNotThrow(() -> SubjectAdapter.getSubject());
Copy link
Contributor

Choose a reason for hiding this comment

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

Use our LambdaTestUtils.intercept() here as it fails better, can validates the type and message

Copy link
Author

Choose a reason for hiding this comment

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

The comment in #7081 (comment) applies here as well.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
import java.io.File;
import java.io.IOException;
import java.lang.reflect.UndeclaredThrowableException;
import java.security.AccessControlContext;
import java.security.AccessController;
import java.security.Principal;
import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
Expand Down Expand Up @@ -90,6 +88,7 @@
import org.apache.hadoop.util.Shell;
import org.apache.hadoop.util.StringUtils;
import org.apache.hadoop.util.Time;
import org.apache.hadoop.util.subject.SubjectAdapter;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -585,8 +584,7 @@ public boolean hasKerberosCredentials() {
@InterfaceStability.Evolving
public static UserGroupInformation getCurrentUser() throws IOException {
ensureInitialized();
AccessControlContext context = AccessController.getContext();
Subject subject = Subject.getSubject(context);
Subject subject = SubjectAdapter.getSubject();
if (subject == null || subject.getPrincipals(User.class).isEmpty()) {
return getLoginUser();
} else {
Expand Down
Loading