From a8d50496752d1435a01c34bb223e7574b6e3241c Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 19 Feb 2023 18:45:34 +0000 Subject: [PATCH 1/3] Support dynamic push credentials obtained from registry --- binderhub/build.py | 39 +++++++++++++++++++++++++++++---------- binderhub/builder.py | 11 +++++++++-- binderhub/registry.py | 8 ++++++++ 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/binderhub/build.py b/binderhub/build.py index d881e0518..da1f3997d 100644 --- a/binderhub/build.py +++ b/binderhub/build.py @@ -91,6 +91,18 @@ class BuildExecutor(LoggingConfigurable): config=True, ) + push_secret_content = Unicode( + "", + help=( + "Content of an implementation dependent secret for pushing image to a registry. " + "For example, if push tokens are temporary this can be used to pass the token " + "as an environment variable CONTAINER_ENGINE_REGISTRY_CREDENTIALS to " + "repo2docker." + "If provided this will be used instead of push_secret." + ), + config=True, + ) + memory_limit = ByteSpecification( 0, help="Memory limit for the build process in bytes (optional suffixes K M G T).", @@ -394,7 +406,23 @@ def submit(self): ) ] - if self.push_secret: + env = [ + client.V1EnvVar(name=key, value=value) + for key, value in self.extra_envs.items() + ] + if self.git_credentials: + env.append( + client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value=self.git_credentials) + ) + + if self.push_secret_content: + env.append( + client.V1EnvVar( + name="CONTAINER_ENGINE_REGISTRY_CREDENTIALS", + value=self.push_secret_content, + ) + ) + elif self.push_secret: volume_mounts.append( client.V1VolumeMount(mount_path="/root/.docker", name="docker-config") ) @@ -405,15 +433,6 @@ def submit(self): ) ) - env = [ - client.V1EnvVar(name=key, value=value) - for key, value in self.extra_envs.items() - ] - if self.git_credentials: - env.append( - client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value=self.git_credentials) - ) - self.pod = client.V1Pod( metadata=client.V1ObjectMeta( name=self.name, diff --git a/binderhub/builder.py b/binderhub/builder.py index 153d24332..ad419c6a3 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -381,11 +381,12 @@ async def get(self, provider_prefix, _unescaped_spec): .lower() ) + image_without_tag, image_tag = _get_image_basename_and_tag(image_name) if self.settings["use_registry"]: for _ in range(3): try: image_manifest = await self.registry.get_image_manifest( - *_get_image_basename_and_tag(image_name) + image_without_tag, image_tag ) image_found = bool(image_manifest) break @@ -457,7 +458,13 @@ async def get(self, provider_prefix, _unescaped_spec): image_name=image_name, git_credentials=provider.git_credentials, ) - if not self.settings["use_registry"]: + if self.settings["use_registry"]: + push_token = await self.registry.get_credentials( + image_without_tag, image_tag + ) + if push_token: + build.push_secret_content = json.dumps(push_token) + else: build.push_secret = "" self.build = build diff --git a/binderhub/registry.py b/binderhub/registry.py index 61721c8f6..0a369334a 100644 --- a/binderhub/registry.py +++ b/binderhub/registry.py @@ -329,6 +329,14 @@ async def get_image_manifest(self, image, tag): raise return json.loads(resp.body.decode("utf-8")) + async def get_credentials(self, image, tag): + """ + If a dynamic token is required for pushing an image to the registry + return a dictionary of login credentials, otherwise return None + (caller should get credentials from some other source) + """ + return None + class FakeRegistry(DockerRegistry): """ From f381eb38f5e4d1e3653b27892accb91c47001e14 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 27 Jun 2023 16:39:39 +0100 Subject: [PATCH 2/3] Replace `push_secret_content` with `registry_credentials` --- binderhub/build.py | 41 +++++++++++++++++++++++++++++++---------- binderhub/builder.py | 2 +- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/binderhub/build.py b/binderhub/build.py index da1f3997d..e0b5bec7c 100644 --- a/binderhub/build.py +++ b/binderhub/build.py @@ -87,18 +87,20 @@ class BuildExecutor(LoggingConfigurable): push_secret = Unicode( "", - help="Implementation dependent secret for pushing image to a registry.", + help="Implementation dependent static secret for pushing image to a registry.", config=True, ) - push_secret_content = Unicode( + registry_credentials = Unicode( "", help=( - "Content of an implementation dependent secret for pushing image to a registry. " - "For example, if push tokens are temporary this can be used to pass the token " - "as an environment variable CONTAINER_ENGINE_REGISTRY_CREDENTIALS to " - "repo2docker." - "If provided this will be used instead of push_secret." + "Implementation dependent credentials for pushing image to a registry. " + "For example, if push tokens are temporary this could be used to pass " + "dynamically created credentials as an encoded JSON blob " + '`{"registry": "docker.io", "username":"user", "password":"password"}` ' + "in the environment variable `CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to " + "repo2docker. " + "If provided this will be used instead of push_secret. " ), config=True, ) @@ -243,7 +245,26 @@ def _default_api(self): # Overrides the default for BuildExecutor push_secret = Unicode( "binder-build-docker-config", - help="Implementation dependent secret for pushing image to a registry.", + help=( + "Name of a Kubernetes secret containing static credentials for pushing " + "an image to a registry." + ), + config=True, + ) + + registry_credentials = Unicode( + "", + help=( + "Implementation dependent credentials for pushing image to a registry. " + "For example, if push tokens are temporary this could be used to pass " + "dynamically created credentials as an encoded JSON blob " + '`{"registry": "docker.io", "username":"user", "password":"password"}` ' + "in the environment variable `CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to " + "repo2docker. " + "If provided this will be used instead of push_secret. " + "Currently this is passed to the build pod as a plan text environment " + "variable, though future implementations may use a Kubernetes secret." + ), config=True, ) @@ -415,11 +436,11 @@ def submit(self): client.V1EnvVar(name="GIT_CREDENTIAL_ENV", value=self.git_credentials) ) - if self.push_secret_content: + if self.registry_credentials: env.append( client.V1EnvVar( name="CONTAINER_ENGINE_REGISTRY_CREDENTIALS", - value=self.push_secret_content, + value=self.registry_credentials, ) ) elif self.push_secret: diff --git a/binderhub/builder.py b/binderhub/builder.py index ad419c6a3..b414b3fe7 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -463,7 +463,7 @@ async def get(self, provider_prefix, _unescaped_spec): image_without_tag, image_tag ) if push_token: - build.push_secret_content = json.dumps(push_token) + build.registry_credentials = json.dumps(push_token) else: build.push_secret = "" From cb6d328dd274133e0dfa78806f39d0dc6823a168 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 27 Jun 2023 22:03:48 +0100 Subject: [PATCH 3/3] Make registry_credentials a Dict --- binderhub/build.py | 30 +++++++++++++++--------------- binderhub/builder.py | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/binderhub/build.py b/binderhub/build.py index e0b5bec7c..019052449 100644 --- a/binderhub/build.py +++ b/binderhub/build.py @@ -91,16 +91,16 @@ class BuildExecutor(LoggingConfigurable): config=True, ) - registry_credentials = Unicode( - "", + registry_credentials = Dict( + {}, help=( "Implementation dependent credentials for pushing image to a registry. " "For example, if push tokens are temporary this could be used to pass " - "dynamically created credentials as an encoded JSON blob " - '`{"registry": "docker.io", "username":"user", "password":"password"}` ' - "in the environment variable `CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to " - "repo2docker. " - "If provided this will be used instead of push_secret. " + "dynamically created credentials " + '`{"registry": "docker.io", "username":"user", "password":"password"}`. ' + "This will be JSON encoded and passed in the environment variable " + "CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to repo2docker. " + "If provided this will be used instead of push_secret." ), config=True, ) @@ -252,17 +252,17 @@ def _default_api(self): config=True, ) - registry_credentials = Unicode( - "", + registry_credentials = Dict( + {}, help=( "Implementation dependent credentials for pushing image to a registry. " "For example, if push tokens are temporary this could be used to pass " - "dynamically created credentials as an encoded JSON blob " - '`{"registry": "docker.io", "username":"user", "password":"password"}` ' - "in the environment variable `CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to " - "repo2docker. " + "dynamically created credentials " + '`{"registry": "docker.io", "username":"user", "password":"password"}`. ' + "This will be JSON encoded and passed in the environment variable " + "CONTAINER_ENGINE_REGISTRY_CREDENTIALS` to repo2docker. " "If provided this will be used instead of push_secret. " - "Currently this is passed to the build pod as a plan text environment " + "Currently this is passed to the build pod as a plain text environment " "variable, though future implementations may use a Kubernetes secret." ), config=True, @@ -440,7 +440,7 @@ def submit(self): env.append( client.V1EnvVar( name="CONTAINER_ENGINE_REGISTRY_CREDENTIALS", - value=self.registry_credentials, + value=json.dumps(self.registry_credentials), ) ) elif self.push_secret: diff --git a/binderhub/builder.py b/binderhub/builder.py index b414b3fe7..4fd34579f 100644 --- a/binderhub/builder.py +++ b/binderhub/builder.py @@ -463,7 +463,7 @@ async def get(self, provider_prefix, _unescaped_spec): image_without_tag, image_tag ) if push_token: - build.registry_credentials = json.dumps(push_token) + build.registry_credentials = push_token else: build.push_secret = ""