Skip to content

Added Harbor config to platform-operator#1099

Open
jordyantunes wants to merge 7 commits intomasterfrom
MLO-471
Open

Added Harbor config to platform-operator#1099
jordyantunes wants to merge 7 commits intomasterfrom
MLO-471

Conversation

@jordyantunes
Copy link
Contributor

No description provided.

@linear
Copy link

linear bot commented Nov 4, 2025

@github-actions github-actions bot added the review-ready PR is ready for the review label Nov 4, 2025
)
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?

- 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?

Copy link
Collaborator

@zubenkoivan zubenkoivan left a comment

Choose a reason for hiding this comment

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

Please let's not merge it, this pr doesn't follow the new way of helm charts configuration that i recently introduced. Let me migrate your changes first.

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?

}

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?

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

Co-authored-by: Yevhenii Semendiak <semendyak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-ready PR is ready for the review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants