-
Notifications
You must be signed in to change notification settings - Fork 551
Use container registry for authenticating kubernetes image pull #3771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
AlexejPenner
wants to merge
16
commits into
develop
Choose a base branch
from
feature/populate-imagepullsecret-from-container-registry
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+742
−43
Open
Changes from 10 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
8bc6e85
Initial commit
AlexejPenner 75622fa
Now also works for (parallel) step pods
AlexejPenner 8f2d890
Reformated
AlexejPenner b8ed918
Removed hint in docs
AlexejPenner 7f983a2
Merge branch 'develop' into feature/populate-imagepullsecret-from-con…
AlexejPenner 064bf5b
Apply first PR review comments
AlexejPenner 2a8bec5
Merge branch 'develop' into feature/populate-imagepullsecret-from-con…
AlexejPenner aeb2bc9
Merge branch 'feature/populate-imagepullsecret-from-container-registr…
AlexejPenner b99472d
apply more reviews
AlexejPenner eeaac76
Readded warning to docs
AlexejPenner 38d7eec
Refactor and adjusted for better doce design
AlexejPenner 0654a4c
Some more refactoring
AlexejPenner 57cc133
Less duplications
AlexejPenner baf0245
Cleaning up docstrings
AlexejPenner ed26186
Merge branch 'develop' into feature/populate-imagepullsecret-from-con…
AlexejPenner 2172a0e
Fixed all broken things
AlexejPenner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,16 +94,13 @@ def requires_authentication(self) -> bool: | |
| def credentials(self) -> Optional[Tuple[str, str]]: | ||
| """Username and password to authenticate with this container registry. | ||
|
|
||
| Service connector credentials take precedence over direct authentication secrets. | ||
|
|
||
| Returns: | ||
| Tuple with username and password if this container registry | ||
| requires authentication, `None` otherwise. | ||
| """ | ||
| secret = self.get_typed_authentication_secret( | ||
| expected_schema_type=BasicAuthSecretSchema | ||
| ) | ||
| if secret: | ||
| return secret.username, secret.password | ||
|
|
||
| # Check service connector credentials first as they take precedence | ||
| connector = self.get_connector() | ||
| if connector: | ||
| from zenml.service_connectors.docker_service_connector import ( | ||
|
|
@@ -116,6 +113,13 @@ def credentials(self) -> Optional[Tuple[str, str]]: | |
| connector.config.password.get_secret_value(), | ||
| ) | ||
|
|
||
| # Fall back to direct authentication secrets | ||
| secret = self.get_typed_authentication_secret( | ||
| expected_schema_type=BasicAuthSecretSchema | ||
| ) | ||
| if secret: | ||
| return secret.username, secret.password | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stefannica according to your comment the connector should be checked first, but in the credentials implementation it was the other way around - let me know if I am missing something or if it made sense to flip here |
||
| return None | ||
|
|
||
| @property | ||
|
|
@@ -232,6 +236,119 @@ def get_image_repo_digest(self, image_name: str) -> Optional[str]: | |
|
|
||
| return cast(str, metadata.id.split(":")[-1]) | ||
|
|
||
| def get_kubernetes_image_pull_secret_data( | ||
| self, | ||
| ) -> Optional[Tuple[str, str, str]]: | ||
| """Get container registry credentials for Kubernetes imagePullSecrets. | ||
|
|
||
| This method only returns credentials when running with a Kubernetes-based | ||
| orchestrator (kubernetes, kubeflow, etc.). For local orchestrators, it | ||
| returns None since imagePullSecrets are not needed. | ||
|
|
||
| Returns: | ||
| Tuple of (registry_uri, username, password) if credentials are available | ||
| and running with a Kubernetes orchestrator, None otherwise. The | ||
| registry_uri is normalized for use in Kubernetes imagePullSecrets. | ||
| """ | ||
| from zenml.client import Client | ||
| from zenml.logger import get_logger | ||
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| # Check if we're using a Kubernetes-based orchestrator | ||
| try: | ||
| stack = Client().active_stack | ||
| orchestrator_flavor = stack.orchestrator.flavor | ||
|
|
||
| # List of orchestrator flavors that use Kubernetes and need imagePullSecrets | ||
| kubernetes_orchestrators = { | ||
| "kubernetes", | ||
| "kubeflow", | ||
| "vertex", | ||
| "sagemaker", | ||
| "tekton", | ||
| "airflow", # when running on Kubernetes | ||
| } | ||
|
|
||
| if orchestrator_flavor not in kubernetes_orchestrators: | ||
| logger.debug( | ||
| f"Skipping ImagePullSecret generation for non-Kubernetes orchestrator: {orchestrator_flavor}" | ||
| ) | ||
| return None | ||
|
|
||
| # Additional check for Kubernetes orchestrator with local flag | ||
| if orchestrator_flavor == "kubernetes" and hasattr( | ||
| stack.orchestrator.config, "local" | ||
| ): | ||
| if stack.orchestrator.config.local: | ||
| logger.debug( | ||
| "Skipping ImagePullSecret generation for local Kubernetes orchestrator" | ||
| ) | ||
| return None | ||
|
|
||
| except Exception as e: | ||
| logger.debug( | ||
| f"Could not determine orchestrator type: {e}. Proceeding with ImagePullSecret generation." | ||
| ) | ||
|
|
||
| logger.debug( | ||
| f"Getting ImagePullSecret data for registry: {self.config.uri}" | ||
| ) | ||
|
|
||
| credentials = self.credentials | ||
| if not credentials: | ||
| logger.debug("No credentials found for container registry") | ||
| return None | ||
|
|
||
| username, password = credentials | ||
| registry_uri = self.config.uri | ||
|
|
||
| logger.debug( | ||
| f"Found credentials - username: {username[:3]}***, registry_uri: {registry_uri}" | ||
| ) | ||
|
|
||
| # Check if there's a service connector with a different registry setting | ||
| connector = self.get_connector() | ||
| if connector: | ||
| from zenml.service_connectors.docker_service_connector import ( | ||
| DockerServiceConnector, | ||
| ) | ||
|
|
||
| if ( | ||
| isinstance(connector, DockerServiceConnector) | ||
| and connector.config.registry | ||
| ): | ||
| # Use the service connector's registry setting | ||
| original_registry_uri = registry_uri | ||
| registry_uri = connector.config.registry | ||
| logger.debug( | ||
| f"Service connector override: {original_registry_uri} -> {registry_uri}" | ||
| ) | ||
|
|
||
| # Normalize registry URI for consistency | ||
| original_registry_uri = registry_uri | ||
| if registry_uri.startswith("https://"): | ||
| registry_uri = registry_uri[8:] | ||
| elif registry_uri.startswith("http://"): | ||
| registry_uri = registry_uri[7:] | ||
|
|
||
| if original_registry_uri != registry_uri: | ||
| logger.debug( | ||
| f"Normalized registry URI: {original_registry_uri} -> {registry_uri}" | ||
| ) | ||
|
|
||
| # Validate the final result | ||
| if not registry_uri or not username or not password: | ||
| logger.warning( | ||
| f"Invalid ImagePullSecret data: registry_uri='{registry_uri}', username='{username}', password_length={len(password) if password else 0}" | ||
| ) | ||
| return None | ||
|
|
||
| logger.debug( | ||
| f"Returning ImagePullSecret data: registry='{registry_uri}', username='{username[:3]}***'" | ||
| ) | ||
| return registry_uri, username, password | ||
|
|
||
|
|
||
| class BaseContainerRegistryFlavor(Flavor): | ||
| """Base flavor for container registries.""" | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.