Skip to content
This repository was archived by the owner on Jan 9, 2020. It is now read-only.

remove camel case naming in kerberos secret names #612

Conversation

aberey
Copy link

@aberey aberey commented Feb 8, 2018

What changes were proposed in this pull request?

This is a bugfix for PR #540 (Basic Secure HDFS Support).

The HadoopKerberosKeytabResolverStep currently uses the constant
KERBEROS_DELEGEGATION_TOKEN_SECRET_NAME when storing the Kerberos
delegation token into a Kubernetes secret - this however causes a
KubernetesClientException stating the following as secret names are supposed to adhere to RFC 1123:

a DNS-1123 subdomain must consist of lower case alphanumeric characters,
'-' or '.', and must start and end with an alphanumeric character (e.g.
'example.com', regex used for validation is
'[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')

The code in question is https://github.com/apache-spark-on-k8s/spark/blob/branch-2.2-kubernetes/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/submitsteps/hadoopsteps/HadoopKerberosKeytabResolverStep.scala#L102

The problem is that the current constants are using camel case, so I have changed the values to not use any uppercase characters anymore.

How was this patch tested?

This was tested manually (against a Kubernetes cluster with version v1.8.5), since the currently implemented unit tests only use mocks and the above Exception is only thrown when the actual Kubernetes API is called.

Copy link
Member

@liyinan926 liyinan926 left a comment

Choose a reason for hiding this comment

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

LGTM.

@liyinan926
Copy link
Member

@aberey can you bring the PR up-to-date with the base branch?

@ifilonenko
Copy link
Member

Thank you for this! @aberey. This should be caught in the integration tests in the integration tests that I am writing.

The names are currently used when HadoopKerberosKeytabResolverStep
tries to safe the kerberos delegation token into a kubernete secret.

However, the current camel case values will cause a
io.fabric8.kubernetes.client.KubernetesClientException
stating the following:

a DNS-1123 subdomain must consist of lower case alphanumeric characters,
'-' or '.', and must start and end with an alphanumeric character (e.g.
'example.com', regex used for validation is
'[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
@aberey aberey force-pushed the kerberos-delegation-token-fix branch from ee1c50e to 0ef937f Compare February 9, 2018 01:09
@aberey
Copy link
Author

aberey commented Feb 9, 2018

@liyinan926 I just rebased the PR on the latest branch-2.2-kubernetes - is there any other branch I should base this on?

@liyinan926
Copy link
Member

Thanks @aberey merging.

@liyinan926 liyinan926 merged commit 7b8c9f5 into apache-spark-on-k8s:branch-2.2-kubernetes Feb 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants