Skip to content

Commit

Permalink
refactor: improve git credential handling in Kubernetes
Browse files Browse the repository at this point in the history
The git credentials are now stored in a temporary secret following recommended
best practice. The secret will have the same lifespan as the builder pod.
  • Loading branch information
sgaist committed Nov 17, 2022
1 parent 0a7c129 commit eb0d9ee
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 21 deletions.
63 changes: 45 additions & 18 deletions binderhub/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Contains build of a docker image from a git repository.
"""

import base64
import datetime
import json
import os
Expand Down Expand Up @@ -382,8 +383,24 @@ def submit(self):

env = []
if self.git_credentials:
secret_content = base64.b64encode(
self.git_credentials.encode("utf-8")
).decode("utf-8")
data = {"credentials": secret_content}

secret = client.V1Secret()
secret.data = data
secret.metadata = {"name": self.name}
secret.type = "Opaque"

self.api.create_namespaced_secret(self.namespace, secret)

secret_key_ref = client.V1SecretKeySelector(
name=self.name, key="credentials", optional=False
)
value_from = client.V1EnvVarSource(secret_key_ref=secret_key_ref)
env.append(
client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value=self.git_credentials)
client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value_from=value_from)
)

self.pod = client.V1Pod(
Expand Down Expand Up @@ -515,10 +532,9 @@ def submit(self):
f"Found unknown phase {phase} when building {self.name}"
)

if self.pod.status.phase == "Succeeded":
self.cleanup()
elif self.pod.status.phase == "Failed":
if self.pod.status.phase in ["Succeeded", "Failed"]:
self.cleanup()

except Exception:
app_log.exception("Error in watch stream for %s", self.name)
raise
Expand Down Expand Up @@ -568,21 +584,32 @@ def stream_logs(self):

def cleanup(self):
"""
Delete the kubernetes build pod
Delete the kubernetes build pod and secret if exists
"""
try:
self.api.delete_namespaced_pod(
name=self.name,
namespace=self.namespace,
body=client.V1DeleteOptions(grace_period_seconds=0),
_request_timeout=KUBE_REQUEST_TIMEOUT,
)
except client.rest.ApiException as e:
if e.status == 404:
# Is ok, someone else has already deleted it
pass
else:
raise

exceptions = []
deletion_methods = [self.api.delete_namespaced_pod]

if self.git_credentials:
deletion_methods.append(self.api.delete_namespaced_secret)

for deletion_method in deletion_methods:
try:
deletion_method(
name=self.name,
namespace=self.namespace,
body=client.V1DeleteOptions(grace_period_seconds=0),
_request_timeout=KUBE_REQUEST_TIMEOUT,
)
except client.rest.ApiException as e:
if e.status == 404:
# Is ok, someone else has already deleted it
pass
else:
exceptions.append(str(e))

if exceptions:
raise RuntimeError("Error(s) occurred during cleanup", exceptions)


class KubernetesCleaner(LoggingConfigurable):
Expand Down
11 changes: 8 additions & 3 deletions binderhub/tests/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,14 @@ def test_git_credentials_passed_to_podspec_upon_submit():

assert len(pod.spec.containers) == 1

env = {env_var.name: env_var.value for env_var in pod.spec.containers[0].env}

assert env["GIT_CREDENTIAL_ENV"] == git_credentials
env = {env_var.name: env_var.value_from for env_var in pod.spec.containers[0].env}

credentials_value = env["GIT_CREDENTIAL_ENV"]
assert isinstance(credentials_value, client.V1EnvVarSource)
assert isinstance(credentials_value.secret_key_ref, client.V1SecretKeySelector)
assert credentials_value.secret_key_ref.key == "credentials"
assert credentials_value.secret_key_ref.name == "test_build"
assert credentials_value.secret_key_ref.optional == False


async def test_local_repo2docker_build():
Expand Down

0 comments on commit eb0d9ee

Please sign in to comment.