Skip to content

Conversation

@dennisvankekem
Copy link
Contributor

@dennisvankekem dennisvankekem commented Jan 6, 2026

@dennisvankekem
Copy link
Contributor Author

based on the assumption that clusterDomainSuffix is always present in settings

Copy link
Collaborator

@ferruhcihan ferruhcihan left a comment

Choose a reason for hiding this comment

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

Overall, the approach looks good 👍 I just noticed that some tests are failing in workloadUtils.test.ts, and there’s a small improvement needed in the URL checks instead of exact match.

: normalizeRepoUrl(repositoryUrl, isPrivate, isSSH)

const repoUrl =
repositoryUrl === `https://gitea.${cluster?.domainSuffix}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This check would never match any actual repository URL and will cause all internal Gitea repositories to fail the equality check. For the URL check, we could use something like the following:

  const isInternalGitea = (() => {
    try {
      const url = new URL(repositoryUrl)
      return url.hostname === `gitea.${cluster?.domainSuffix}`
    } catch {
      return false
    }
  })()

  const repoUrl = isInternalGitea
    ? repositoryUrl
    : normalizeRepoUrl(repositoryUrl, isPrivate, isSSH)

post:
operationId: getWorkloadCatalog
x-eov-operation-handler: v1/workloadCatalog
x-aclSchema: Workload
Copy link
Contributor

Choose a reason for hiding this comment

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

Why schema is called workload if the resource is tWorkloadCatalog?

if (!existsSync(localHelmChartsDir)) mkdirSync(localHelmChartsDir, { recursive: true })
let gitUrl = helmChartCatalogUrl
if (isGiteaURL(helmChartCatalogUrl)) {
if (helmChartCatalogUrl === `https://gitea.${clusterDomainSuffix}`) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you could also rely on HELM_CHART_CATALOG env

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants