You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by ambud <gi...@git.apache.org> on 2017/04/10 23:55:56 UTC
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
GitHub user ambud opened a pull request:
https://github.com/apache/storm/pull/2054
STORM-2462 Adding regex mapper to KerberosPrincipalToLocal class
In some environments it may be the case that the local unix usernames are different than the ones in Kerberos account. This feature lets you override the username and map it to the one locally available.
https://issues.apache.org/jira/browse/STORM-2462
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ambud/storm master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2054.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2054
----
commit 25221633b5267a09c7d7bd911f509a0f08467c44
Author: ambud <as...@gmail.com>
Date: 2017-04-10T23:55:03Z
STORM-2462 Adding regex mapper to KerberosPrincipalToLocal class
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2054: STORM-2462 Adding regex mapper to KerberosPrincipalToLoca...
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on the issue:
https://github.com/apache/storm/pull/2054
@ambud we should add the auth_to_local rules an make that as part of storm.yaml config option . Adding regex will not be helpful here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2054: STORM-2462 Adding regex mapper to KerberosPrincipalToLoca...
Posted by ambud <gi...@git.apache.org>.
Github user ambud commented on the issue:
https://github.com/apache/storm/pull/2054
Feature no longer needed
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r111191074
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
--- End diff --
Really there are two ways to make this optional. You can do it like this and making it a separate class or you can have the configs be optional, and if they are not set then no modification is done. I kind of prefer the other one, as it feels more intuitive to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110968735
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
+
+ private String inputRegex;
+ private String replacementString;
+
+ /**
+ * Invoked once immediately after construction
+ * @param storm_conf Storm configuration
+ */
+ public void prepare(@SuppressWarnings("rawtypes") Map storm_conf) {
+ Object object = storm_conf.get("storm.principal.mapper.regex");
+ if (object != null) {
+ inputRegex = object.toString();
+ }
+ object = storm_conf.get("storm.principal.mapper.replacement");
--- End diff --
Could we put all of the configs here inside of Config.java and then tag them with the annotations to make it a string so we don't need to call .toString on the things passed in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110967560
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
+
+ private String inputRegex;
+ private String replacementString;
--- End diff --
nit: The indentation her and in general in some of the files appears to be off.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2054: STORM-2462 Adding regex mapper to KerberosPrincipalToLoca...
Posted by ambud <gi...@git.apache.org>.
Github user ambud commented on the issue:
https://github.com/apache/storm/pull/2054
@harshach got it, let me work on the implementation and update the code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110968429
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
--- End diff --
I would prefer to see this be inside KerberosPrincipalToLocal instead of having a separate class.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by ambud <gi...@git.apache.org>.
Github user ambud commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110976923
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
+
+ private String inputRegex;
+ private String replacementString;
+
+ /**
+ * Invoked once immediately after construction
+ * @param storm_conf Storm configuration
+ */
+ public void prepare(@SuppressWarnings("rawtypes") Map storm_conf) {
+ Object object = storm_conf.get("storm.principal.mapper.regex");
+ if (object != null) {
+ inputRegex = object.toString();
+ }
+ object = storm_conf.get("storm.principal.mapper.replacement");
--- End diff --
Can we do conditional not null checks in Config.java? @revans2
```
@isString
@NotNull
public static final String STORM_PRINCIPAL_MAPPER_REGEX = "storm.principal.mapper.regex";
@isString
@NotNull
public static final String STORM_PRINCIPAL_MAPPER_REPLACEMENT = "storm.principal.mapper.replacement";
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by ambud <gi...@git.apache.org>.
Github user ambud commented on a diff in the pull request:
https://github.com/apache/storm/pull/2054#discussion_r110977694
--- Diff: storm-client/src/jvm/org/apache/storm/security/auth/RegexKerberosPrincipalToLocal.java ---
@@ -0,0 +1,56 @@
+/**
+ * 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.storm.security.auth;
+
+import java.util.Map;
+import java.security.Principal;
+
+/**
+ * Map a kerberos principal to a local user
+ */
+public class RegexKerberosPrincipalToLocal extends KerberosPrincipalToLocal {
--- End diff --
This implementation is useful for a few environments that have complications of usename mapping.
The standard KerberosPrincipalToLocal works for majority of the environments so my approach was to not alter it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm issue #2054: STORM-2462 Adding regex mapper to KerberosPrincipalToLoca...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2054
I have been thinking about this, and I am not sure if a REGEX is really what we want. Hadoop does this in a semi-standard way using auth_to_local rules like PAM does.
It might be worth lifting some of that code from hadoop and reusing it here instead of just a regular expression.
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/KerberosName.java
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request #2054: STORM-2462 Adding regex mapper to KerberosPrincipa...
Posted by ambud <gi...@git.apache.org>.
Github user ambud closed the pull request at:
https://github.com/apache/storm/pull/2054
---