Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions charts/platform-operator/crds/platform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ spec:
type: object
required: [persistence]
properties:
registry_type:
type: string
enum:
- "docker"
- "harbor"
default: "docker"
persistence:
type: object
required: [storageClassName]
Expand Down
4 changes: 4 additions & 0 deletions charts/platform/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ dependencies:
version: "1.9.2"
repository: https://charts.helm.sh/stable
condition: dockerRegistryEnabled
- name: harbor
version: "1.17.0"
repository: https://helm.goharbor.io
condition: harborEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need a condition for harbor?

- name: minio
version: "5.0.33"
repository: https://charts.helm.sh/stable
Expand Down
79 changes: 76 additions & 3 deletions platform_operator/helm_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from .models import (
BucketsProvider,
DockerRegistryStorageDriver,
DockerRegistryType,
HelmChartNames,
IngressServiceType,
LabelsConfig,
Expand Down Expand Up @@ -62,6 +63,7 @@ def create_platform_values(self, platform: PlatformConfig) -> dict[str, Any]:
"traefikEnabled": platform.ingress_controller_install,
"acmeEnabled": platform.ingress_acme_enabled,
"dockerRegistryEnabled": platform.registry.docker_registry_install,
"harborEnabled": platform.registry.harbor_registry_install,
"appsPostgresOperatorEnabled": (
platform.apps_operator_config.postgres_operator_enabled
),
Expand Down Expand Up @@ -198,9 +200,13 @@ def create_platform_values(self, platform: PlatformConfig) -> dict[str, Any]:
else:
result["dockerHubConfigSecret"] = {"create": False}
if platform.registry.docker_registry_install:
result[HelmChartNames.docker_registry] = self.create_docker_registry_values(
platform
)
if platform.registry.docker_registry_type == DockerRegistryType.DOCKER:
result[HelmChartNames.docker_registry] = (
self.create_docker_registry_values(platform)
)
elif platform.registry.docker_registry_type == DockerRegistryType.HARBOR:
result[HelmChartNames.harbor] = self.create_harbor_values(platform)

if platform.buckets.minio_install:
assert platform.buckets.minio_public_url
result["ingress"]["minioHost"] = platform.buckets.minio_public_url.host
Expand Down Expand Up @@ -411,6 +417,73 @@ def create_docker_registry_values(self, platform: PlatformConfig) -> dict[str, A
}
return result

def create_harbor_values(self, platform: PlatformConfig) -> dict[str, Any]:
docker_registry_credentials_secret_name = (
f"{platform.release_name}-docker-registry"
)
harbor_external_host = f"harbor.apps.{platform.ingress_dns_name}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why harbor.apps...? It's not an app

result: dict[str, Any] = {
"expose": {
"tls": {"enabled": False},
"ingress": {
"hosts": {"core": harbor_external_host},
"className": "traefik",
"annotations": None,
},
},
"persistence": {
"persistentVolumeClaim": {
"registry": {},
},
"imageChartStorage": {},
},
"externalURL": harbor_external_host,
}

if (
platform.registry.docker_registry_username
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to save images to docker registry?

and platform.registry.docker_registry_password
):
assert platform.registry.docker_registry_username == "admin"
result["existingSecretAdminPassword"] = (
docker_registry_credentials_secret_name
)
result["existingSecretAdminPasswordKey"] = "password"

if (
platform.registry.docker_registry_storage_driver
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to support persistence to disk?

== DockerRegistryStorageDriver.FILE_SYSTEM
):
result["persistence"]["imageChartStorage"]["type"] = "filesystem"
result["persistence"]["persistentVolumeClaim"]["registry"] = {
"storageClass": (
platform.registry.docker_registry_file_system_storage_class_name
),
"size": platform.registry.docker_registry_file_system_storage_size,
}
elif (
platform.registry.docker_registry_storage_driver
== DockerRegistryStorageDriver.S3
):
assert platform.registry.docker_registry_s3_endpoint
result["persistence"]["imageChartStorage"]["type"] = "s3"
result["persistence"]["imageChartStorage"]["s3"] = {
"bucket": str(platform.registry.docker_registry_s3_bucket),
"region": platform.registry.docker_registry_s3_region,
"regionendpoint": self._get_url_authority(
platform.registry.docker_registry_s3_endpoint
),
"accesskey": platform.registry.docker_registry_s3_access_key,
"secretkey": platform.registry.docker_registry_s3_secret_key,
"secure": platform.registry.docker_registry_s3_endpoint.scheme
== "https",
"rootdirectory": "/harbor",
}
result["persistence"]["imageChartStorage"]["disableredirect"] = (
platform.registry.docker_registry_s3_disable_redirect
)
return result

def create_minio_values(self, platform: PlatformConfig) -> dict[str, Any]:
assert platform.buckets.minio_public_url
return {
Expand Down
31 changes: 24 additions & 7 deletions platform_operator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class HelmReleaseNames:

class HelmChartNames:
docker_registry: str = "docker-registry"
harbor: str = "harbor"
minio: str = "minio"
minio_gateway: str = "minio-gateway"
traefik: str = "traefik"
Expand Down Expand Up @@ -596,6 +597,11 @@ class DockerRegistryStorageDriver(str, Enum):
S3 = "s3"


class DockerRegistryType(str, Enum):
DOCKER = "docker"
HARBOR = "harbor"


@dataclass(frozen=True)
class RegistryConfig:
provider: RegistryProvider
Expand All @@ -610,6 +616,7 @@ class RegistryConfig:
azure_password: str = ""

docker_registry_install: bool = False
docker_registry_type: DockerRegistryType = DockerRegistryType.DOCKER
docker_registry_url: URL | None = None
docker_registry_username: str = ""
docker_registry_password: str = ""
Expand Down Expand Up @@ -1035,7 +1042,7 @@ def create(self, platform_body: kopf.Body, cluster: Cluster) -> PlatformConfig:
idle_jobs=cluster.orchestrator.idle_jobs,
buckets=buckets_config,
registry=self._create_registry(
spec.registry, buckets_config=buckets_config
spec.registry, buckets_config=buckets_config, cluster=cluster
),
monitoring=self._create_monitoring(spec.monitoring),
disks_storage_limit_per_user=cluster.disks.storage_limit_per_user,
Expand Down Expand Up @@ -1222,7 +1229,7 @@ def _create_buckets(self, spec: BlobStorageSpec, cluster: Cluster) -> BucketsCon
raise ValueError("Bucket provider is not supported")

def _create_registry(
self, spec: RegistrySpec, *, buckets_config: BucketsConfig
self, spec: RegistrySpec, *, buckets_config: BucketsConfig, cluster: Cluster
) -> RegistryConfig:
if not spec:
raise ValueError("Registry spec is empty")
Expand Down Expand Up @@ -1256,14 +1263,24 @@ def _create_registry(
docker_registry_password=spec.docker_password,
)
if "kubernetes" in spec:
return RegistryConfig(
provider=RegistryProvider.DOCKER,
docker_registry_install=True,
docker_registry_url=URL.build(
reg_type = DockerRegistryType(
spec.get("kubernetes", {}).get("registry_type", "docker")
)
if reg_type == DockerRegistryType.HARBOR:
docker_registry_url = URL.build(
scheme="https", host=f"harbor.apps.{cluster.dns.name}"
Copy link
Contributor

Choose a reason for hiding this comment

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

why not in-cluster service hostname?

)
else:
docker_registry_url = URL.build(
scheme="http",
host=f"{self._config.helm_release_names.platform}-docker-registry",
port=5000,
),
)
return RegistryConfig(
provider=RegistryProvider.DOCKER,
docker_registry_install=True,
docker_registry_type=reg_type,
docker_registry_url=docker_registry_url,
docker_registry_storage_driver=DockerRegistryStorageDriver.FILE_SYSTEM,
docker_registry_file_system_storage_class_name=(
spec.kubernetes_storage_class_name
Expand Down
146 changes: 146 additions & 0 deletions tests/unit/test_helm_values.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
BucketsProvider,
DockerConfig,
DockerRegistryStorageDriver,
DockerRegistryType,
ExternalSecretObjectsSpec,
ExternalSecretsSpec,
IngressServiceType,
Expand All @@ -39,6 +40,7 @@ def test_create_gcp_platform_values(
"traefikEnabled": True,
"acmeEnabled": True,
"dockerRegistryEnabled": False,
"harborEnabled": False,
"appsPostgresOperatorEnabled": True,
"appsSparkOperatorEnabled": True,
"appsKedaEnabled": True,
Expand Down Expand Up @@ -356,6 +358,42 @@ def test_create_on_prem_platform_values_without_docker_registry(
assert result["dockerRegistryEnabled"] is False
assert "docker-registry" not in result

def test_create_on_prem_platform_values_with_harbor_registry(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
result = factory.create_platform_values(
replace(
on_prem_platform_config,
registry=replace(
on_prem_platform_config.registry,
docker_registry_type=DockerRegistryType.HARBOR,
),
)
)

assert result["dockerRegistryEnabled"] is False
assert result["harborEnabled"] is True
assert "harbor" in result
assert "docker-registry" not in result

def test_create_on_prem_platform_values_with_docker_registry_explicit(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
result = factory.create_platform_values(
replace(
on_prem_platform_config,
registry=replace(
on_prem_platform_config.registry,
docker_registry_type=DockerRegistryType.DOCKER,
),
)
)

assert result["dockerRegistryEnabled"] is True
assert result["harborEnabled"] is False
assert "docker-registry" in result
assert "harbor" not in result

def test_create_on_prem_platform_values_without_minio(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
Expand Down Expand Up @@ -515,6 +553,114 @@ def test_create_docker_registry_values_with_s3_minio_storage(
}
assert result["secrets"]["haSharedSecret"]

def test_create_harbor_values_with_filesystem_storage(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
on_prem_platform_config = replace(
on_prem_platform_config,
registry=replace(
on_prem_platform_config.registry,
docker_registry_type=DockerRegistryType.HARBOR,
),
)
result = factory.create_harbor_values(on_prem_platform_config)

assert result == {
"expose": {
"tls": {"enabled": False},
"ingress": {
"hosts": {
"core": "harbor.apps."
f"{on_prem_platform_config.ingress_dns_name}"
},
"className": "traefik",
"annotations": None,
},
},
"persistence": {
"persistentVolumeClaim": {
"registry": {
"storageClass": "registry-standard",
"size": "100Gi",
},
},
"imageChartStorage": {"type": "filesystem"},
},
"externalURL": f"harbor.apps.{on_prem_platform_config.ingress_dns_name}",
}

def test_create_harbor_values_with_s3_storage(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
on_prem_platform_config = replace(
on_prem_platform_config,
registry=replace(
on_prem_platform_config.registry,
docker_registry_type=DockerRegistryType.HARBOR,
docker_registry_storage_driver=DockerRegistryStorageDriver.S3,
docker_registry_s3_endpoint=URL("https://s3.amazonaws.com"),
docker_registry_s3_region="us-east-1",
docker_registry_s3_bucket="harbor-registry",
docker_registry_s3_access_key="aws-access-key",
docker_registry_s3_secret_key="aws-secret-key",
docker_registry_s3_disable_redirect=False,
),
)
result = factory.create_harbor_values(on_prem_platform_config)

assert result == {
"expose": {
"tls": {"enabled": False},
"ingress": {
"hosts": {
"core": "harbor.apps."
f"{on_prem_platform_config.ingress_dns_name}"
},
"className": "traefik",
"annotations": None,
},
},
"persistence": {
"persistentVolumeClaim": {
"registry": {},
},
"imageChartStorage": {
"type": "s3",
"s3": {
"bucket": "harbor-registry",
"region": "us-east-1",
"regionendpoint": "s3.amazonaws.com",
"accesskey": "aws-access-key",
"secretkey": "aws-secret-key",
"secure": True,
"rootdirectory": "/harbor",
},
"disableredirect": False,
},
},
"externalURL": f"harbor.apps.{on_prem_platform_config.ingress_dns_name}",
}

def test_create_harbor_values_with_admin_credentials(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
on_prem_platform_config = replace(
on_prem_platform_config,
registry=replace(
on_prem_platform_config.registry,
docker_registry_type=DockerRegistryType.HARBOR,
docker_registry_username="admin",
docker_registry_password="admin-password",
),
)
result = factory.create_harbor_values(on_prem_platform_config)

assert (
result["existingSecretAdminPassword"]
== f"{on_prem_platform_config.release_name}-docker-registry"
)
assert result["existingSecretAdminPasswordKey"] == "password"

def test_create_minio_values(
self, on_prem_platform_config: PlatformConfig, factory: HelmValuesFactory
) -> None:
Expand Down
Loading
Loading