Skip to content

fix: missing acl schema #888

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.

Sign up for GitHub

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
dennisvankekem wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: missing acl schema #888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a674542
fix: missing acl schema
dennisvankekem Jan 6, 2026
c8f38be
fix: added internal gitea check
dennisvankekem Jan 6, 2026
7cdfa90
fix: reverted giteaurl removal
dennisvankekem Jan 6, 2026
507625e
Merge remote-tracking branch 'origin/main' into APL-1412
svcAPLBot Jan 8, 2026
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
3 changes: 3 additions & 0 deletions src/openapi/api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1991,6 +1991,7 @@ paths:
post:
operationId: getWorkloadCatalog
x-eov-operation-handler: v1/workloadCatalog
x-aclSchema: Workload
Copy link
Contributor

@j-zimnowoda j-zimnowoda Jan 7, 2026

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?

Copy link
Contributor Author

@dennisvankekem dennisvankekem Jan 9, 2026

Choose a reason for hiding this comment

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

WorkloadCatalog is part of the Workload workflow

description: Get workload catalog from a repository
requestBody:
content:
Expand All @@ -2015,6 +2016,7 @@ paths:
get:
operationId: getHelmChartContent
x-eov-operation-handler: v1/helmChartContent
x-aclSchema: Workload
parameters:
- name: url
in: query
Expand All @@ -2041,6 +2043,7 @@ paths:
post:
operationId: createWorkloadCatalog
x-eov-operation-handler: v1/createWorkloadCatalog
x-aclSchema: Workload
description: Create workload catalog from a repository
requestBody:
content:
Expand Down
16 changes: 11 additions & 5 deletions src/otomi-stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CoreV1Api, KubeConfig, User as k8sUser, V1ObjectReference } from '@kubernetes/client-node'
import { CoreV1Api, User as k8sUser, KubeConfig, V1ObjectReference } from '@kubernetes/client-node'
import Debug from 'debug'

import { getRegions, ObjectStorageKeyRegions } from '@linode/api-v4'
Expand Down Expand Up @@ -1210,6 +1210,7 @@ export default class OtomiStack {
if (!codeRepoName) return ['HEAD']
const coderepo = this.getCodeRepo(teamId, codeRepoName)
const { repositoryUrl, secret: secretName } = coderepo
const { cluster } = this.getSettings(['cluster'])
try {
let sshPrivateKey = '',
username = '',
Expand All @@ -1224,9 +1225,11 @@ export default class OtomiStack {

const isPrivate = !!secretName
const isSSH = !!sshPrivateKey
const repoUrl = repositoryUrl.startsWith('https://gitea')
? repositoryUrl
: normalizeRepoUrl(repositoryUrl, isPrivate, isSSH)

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

@ferruhcihan ferruhcihan Jan 7, 2026

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)

Copy link
Contributor Author

@dennisvankekem dennisvankekem Jan 9, 2026

Choose a reason for hiding this comment

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

I will add an isInternalGiteaURL function and replace the duplicated checks with this function call. I wil also make a test for it

Comment on lines +1229 to +1230
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

When cluster?.domainSuffix is undefined, this creates the string 'https://gitea.undefined' which will cause incorrect URL matching. Add a guard to check cluster?.domainSuffix exists before string interpolation.

Suggested change
const repoUrl =
repositoryUrl === `https://gitea.${cluster?.domainSuffix}`
const giteaUrl = cluster?.domainSuffix ? `https://gitea.${cluster.domainSuffix}` : undefined
const repoUrl =
giteaUrl && repositoryUrl === giteaUrl

Copilot uses AI. Check for mistakes.
? repositoryUrl
: normalizeRepoUrl(repositoryUrl, isPrivate, isSSH)

if (!repoUrl) return ['HEAD']

Expand Down Expand Up @@ -1566,8 +1569,9 @@ export default class OtomiStack {

if (!url) throw new OtomiError(400, 'Helm chart catalog URL is not set')

const { cluster } = this.getSettings(['cluster'])
try {
const { helmCharts, catalog } = await fetchWorkloadCatalog(url, helmChartsDir, teamId)
const { helmCharts, catalog } = await fetchWorkloadCatalog(url, helmChartsDir, teamId, cluster?.domainSuffix)
return { url, helmCharts, catalog }
} catch (error) {
debug('Error fetching workload catalog')
Expand All @@ -1588,6 +1592,7 @@ export default class OtomiStack {
const localHelmChartsDir = `/tmp/otomi/charts/${uuid}`
const helmChartCatalogUrl = env.HELM_CHART_CATALOG
const { user, email } = this.git
const { cluster } = this.getSettings(['cluster'])

try {
await sparseCloneChart(
Expand All @@ -1599,6 +1604,7 @@ export default class OtomiStack {
chartTargetDirName,
chartIcon,
allowTeams,
cluster?.domainSuffix,
)
return true
} catch (error) {
Expand Down
12 changes: 9 additions & 3 deletions src/utils/workloadUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export async function sparseCloneChart(
chartTargetDirName: string,
chartIcon?: string,
allowTeams?: boolean,
clusterDomainSuffix?: string,
): Promise<boolean> {
const details = detectGitProvider(gitRepositoryUrl)
if (!details) return false
Expand All @@ -278,7 +279,7 @@ export async function sparseCloneChart(

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

@j-zimnowoda j-zimnowoda Jan 7, 2026

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

Copy link
Contributor Author

@dennisvankekem dennisvankekem Jan 9, 2026

Choose a reason for hiding this comment

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

good tip! I will take it into account

Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The URL comparison will fail when clusterDomainSuffix is undefined. The condition should check if clusterDomainSuffix exists before comparing, or use startsWith to match the URL pattern more robustly.

Suggested change
if (helmChartCatalogUrl === `https://gitea.${clusterDomainSuffix}`) {
if (clusterDomainSuffix && helmChartCatalogUrl.startsWith(`https://gitea.${clusterDomainSuffix}`)) {

Copilot uses AI. Check for mistakes.
const [protocol, bareUrl] = helmChartCatalogUrl.split('://')
const encodedUser = encodeURIComponent(process.env.GIT_USER as string)
const encodedPassword = encodeURIComponent(process.env.GIT_PASSWORD as string)
Expand Down Expand Up @@ -318,10 +319,15 @@ export async function sparseCloneChart(
return true
}

export async function fetchWorkloadCatalog(url: string, helmChartsDir: string, teamId: string): Promise<Promise<any>> {
export async function fetchWorkloadCatalog(
url: string,
helmChartsDir: string,
teamId: string,
clusterDomainSuffix?: string,
): Promise<Promise<any>> {
if (!existsSync(helmChartsDir)) mkdirSync(helmChartsDir, { recursive: true })
let gitUrl = url
if (isGiteaURL(url)) {
if (url === `https://gitea.${clusterDomainSuffix}`) {
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The URL comparison will fail when clusterDomainSuffix is undefined. The condition should check if clusterDomainSuffix exists before comparing, or use startsWith to match the URL pattern more robustly.

Suggested change
if (url === `https://gitea.${clusterDomainSuffix}`) {
if (clusterDomainSuffix && url.startsWith(`https://gitea.${clusterDomainSuffix}`)) {

Copilot uses AI. Check for mistakes.
const [protocol, bareUrl] = url.split('://')
const encodedUser = encodeURIComponent(process.env.GIT_USER as string)
const encodedPassword = encodeURIComponent(process.env.GIT_PASSWORD as string)
Expand Down
Loading