From 48d9b170cb563420e0d351d976fd11f5959dd8b6 Mon Sep 17 00:00:00 2001 From: Feroze Mohideen Date: Tue, 19 Sep 2023 11:18:37 -0400 Subject: [PATCH 1/2] send app name to parse porter.yaml request in case the porter.yaml version is v1 --- dashboard/src/lib/hooks/usePorterYaml.ts | 11 ++- .../app-view/LatestRevisionContext.tsx | 1 + .../app-dashboard/create-app/CreateApp.tsx | 7 +- dashboard/src/shared/api.tsx | 84 ++++++++----------- internal/porter_app/v2/yaml.go | 4 + 5 files changed, 53 insertions(+), 54 deletions(-) diff --git a/dashboard/src/lib/hooks/usePorterYaml.ts b/dashboard/src/lib/hooks/usePorterYaml.ts index 8550f68f8f..f0e3204b75 100644 --- a/dashboard/src/lib/hooks/usePorterYaml.ts +++ b/dashboard/src/lib/hooks/usePorterYaml.ts @@ -26,13 +26,17 @@ type PorterYamlStatus = * usePorterYaml is a hook that will fetch the porter.yaml file from the * specified source and parse it to determine the services that should be * added to an app by default with read-only values. - * + * + * `appName` is required to be passed in to accommodate v1 porter.yaml, since v1 porter.yaml does not explicitly specify the app name. + * if the porter.yaml is v2, and the file contains an app name, then the appName parameter here will be ignored. */ export const usePorterYaml = ({ source, + appName = "", useDefaults = true, }: { source: (SourceOptions & { type: "github" }) | null; + appName?: string; useDefaults?: boolean; }): PorterYamlStatus => { const { currentProject, currentCluster } = useContext(Context); @@ -95,15 +99,17 @@ export const usePorterYaml = ({ b64Yaml, projectId, clusterId, + appName, }: { b64Yaml: string; projectId: number; clusterId: number; + appName: string; }) => { try { const res = await api.parsePorterYaml( "", - { b64_yaml: b64Yaml }, + { b64_yaml: b64Yaml, app_name: appName }, { project_id: projectId, cluster_id: clusterId, @@ -149,6 +155,7 @@ export const usePorterYaml = ({ b64Yaml: data, projectId: currentProject.id, clusterId: currentCluster.id, + appName, }); } diff --git a/dashboard/src/main/home/app-dashboard/app-view/LatestRevisionContext.tsx b/dashboard/src/main/home/app-dashboard/app-view/LatestRevisionContext.tsx index 8dacfe1d9e..c05e07630d 100644 --- a/dashboard/src/main/home/app-dashboard/app-view/LatestRevisionContext.tsx +++ b/dashboard/src/main/home/app-dashboard/app-view/LatestRevisionContext.tsx @@ -145,6 +145,7 @@ export const LatestRevisionProvider = ({ const { loading: porterYamlLoading, detectedServices } = usePorterYaml({ source: latestSource?.type === "github" ? latestSource : null, + appName: porterApp?.name, useDefaults: false, }); diff --git a/dashboard/src/main/home/app-dashboard/create-app/CreateApp.tsx b/dashboard/src/main/home/app-dashboard/create-app/CreateApp.tsx index cb20efedbb..9475e67c7d 100644 --- a/dashboard/src/main/home/app-dashboard/create-app/CreateApp.tsx +++ b/dashboard/src/main/home/app-dashboard/create-app/CreateApp.tsx @@ -183,7 +183,7 @@ const CreateApp: React.FC = ({ history }) => { porterYamlFound, detectedName, loading: isLoadingPorterYaml, - } = usePorterYaml({ source: source?.type === "github" ? source : null }); + } = usePorterYaml({ source: source?.type === "github" ? source : null, appName: name.value }); const deploymentTarget = useDefaultDeploymentTarget(); const { updateAppStep } = useAppAnalytics(name.value); const { validateApp } = useAppValidation({ @@ -585,9 +585,8 @@ const CreateApp: React.FC = ({ history }) => { } > {detectedServices.count > 0 - ? `Detected ${detectedServices.count} service${ - detectedServices.count > 1 ? "s" : "" - } from porter.yaml.` + ? `Detected ${detectedServices.count} service${detectedServices.count > 1 ? "s" : "" + } from porter.yaml.` : `Could not detect any services from porter.yaml. Make sure it exists in the root of your repo.`} diff --git a/dashboard/src/shared/api.tsx b/dashboard/src/shared/api.tsx index 0f2c83b960..7d698debf9 100644 --- a/dashboard/src/shared/api.tsx +++ b/dashboard/src/shared/api.tsx @@ -320,9 +320,8 @@ const getFeedEvents = baseApi< } >("GET", (pathParams) => { let { project_id, cluster_id, stack_name, page } = pathParams; - return `/api/projects/${project_id}/clusters/${cluster_id}/applications/${stack_name}/events?page=${ - page || 1 - }`; + return `/api/projects/${project_id}/clusters/${cluster_id}/applications/${stack_name}/events?page=${page || 1 + }`; }); const createEnvironment = baseApi< @@ -747,11 +746,9 @@ const detectBuildpack = baseApi< branch: string; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.project_id}/gitrepos/${ - pathParams.git_repo_id - }/repos/${pathParams.kind}/${pathParams.owner}/${ - pathParams.name - }/${encodeURIComponent(pathParams.branch)}/buildpack/detect`; + return `/api/projects/${pathParams.project_id}/gitrepos/${pathParams.git_repo_id + }/repos/${pathParams.kind}/${pathParams.owner}/${pathParams.name + }/${encodeURIComponent(pathParams.branch)}/buildpack/detect`; }); const detectGitlabBuildpack = baseApi< @@ -782,11 +779,9 @@ const getBranchContents = baseApi< branch: string; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.project_id}/gitrepos/${ - pathParams.git_repo_id - }/repos/${pathParams.kind}/${pathParams.owner}/${ - pathParams.name - }/${encodeURIComponent(pathParams.branch)}/contents`; + return `/api/projects/${pathParams.project_id}/gitrepos/${pathParams.git_repo_id + }/repos/${pathParams.kind}/${pathParams.owner}/${pathParams.name + }/${encodeURIComponent(pathParams.branch)}/contents`; }); const getProcfileContents = baseApi< @@ -802,11 +797,9 @@ const getProcfileContents = baseApi< branch: string; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.project_id}/gitrepos/${ - pathParams.git_repo_id - }/repos/${pathParams.kind}/${pathParams.owner}/${ - pathParams.name - }/${encodeURIComponent(pathParams.branch)}/procfile`; + return `/api/projects/${pathParams.project_id}/gitrepos/${pathParams.git_repo_id + }/repos/${pathParams.kind}/${pathParams.owner}/${pathParams.name + }/${encodeURIComponent(pathParams.branch)}/procfile`; }); const getPorterYamlContents = baseApi< @@ -822,16 +815,15 @@ const getPorterYamlContents = baseApi< branch: string; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.project_id}/gitrepos/${ - pathParams.git_repo_id - }/repos/${pathParams.kind}/${pathParams.owner}/${ - pathParams.name - }/${encodeURIComponent(pathParams.branch)}/porteryaml`; + return `/api/projects/${pathParams.project_id}/gitrepos/${pathParams.git_repo_id + }/repos/${pathParams.kind}/${pathParams.owner}/${pathParams.name + }/${encodeURIComponent(pathParams.branch)}/porteryaml`; }); const parsePorterYaml = baseApi< { b64_yaml: string; + app_name: string; }, { project_id: number; @@ -862,11 +854,9 @@ const getBranchHead = baseApi< branch: string; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.project_id}/gitrepos/${ - pathParams.git_repo_id - }/repos/${pathParams.kind}/${pathParams.owner}/${ - pathParams.name - }/${encodeURIComponent(pathParams.branch)}/head`; + return `/api/projects/${pathParams.project_id}/gitrepos/${pathParams.git_repo_id + }/repos/${pathParams.kind}/${pathParams.owner}/${pathParams.name + }/${encodeURIComponent(pathParams.branch)}/head`; }); const validatePorterApp = baseApi< @@ -890,21 +880,21 @@ const validatePorterApp = baseApi< const createApp = baseApi< | { - name: string; - type: "github"; - git_repo_id: number; - git_branch: string; - git_repo_name: string; - porter_yaml_path: string; - } + name: string; + type: "github"; + git_repo_id: number; + git_branch: string; + git_repo_name: string; + porter_yaml_path: string; + } | { - name: string; - type: "docker-registry"; - image: { - repository: string; - tag: string; - }; - }, + name: string; + type: "docker-registry"; + image: { + repository: string; + tag: string; + }; + }, { project_id: number; cluster_id: number; @@ -1894,11 +1884,9 @@ const getEnvGroup = baseApi< version?: number; } >("GET", (pathParams) => { - return `/api/projects/${pathParams.id}/clusters/${ - pathParams.cluster_id - }/namespaces/${pathParams.namespace}/envgroup?name=${pathParams.name}${ - pathParams.version ? "&version=" + pathParams.version : "" - }`; + return `/api/projects/${pathParams.id}/clusters/${pathParams.cluster_id + }/namespaces/${pathParams.namespace}/envgroup?name=${pathParams.name}${pathParams.version ? "&version=" + pathParams.version : "" + }`; }); const getConfigMap = baseApi< @@ -2955,7 +2943,7 @@ const removeStackEnvGroup = baseApi< `/api/v1/projects/${project_id}/clusters/${cluster_id}/namespaces/${namespace}/stacks/${stack_id}/remove_env_group/${env_group_name}` ); -const getGithubStatus = baseApi<{}, {}>("GET", ({}) => `/api/status/github`); +const getGithubStatus = baseApi<{}, {}>("GET", ({ }) => `/api/status/github`); const createSecretAndOpenGitHubPullRequest = baseApi< { diff --git a/internal/porter_app/v2/yaml.go b/internal/porter_app/v2/yaml.go index 23ac6accb9..c508cbe551 100644 --- a/internal/porter_app/v2/yaml.go +++ b/internal/porter_app/v2/yaml.go @@ -35,6 +35,10 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte, appName strin Name: porterYaml.Name, } + if appProto.Name == "" { + return nil, nil, telemetry.Error(ctx, span, nil, "app name is empty") + } + if porterYaml.Build != nil { appProto.Build = &porterv1.Build{ Context: porterYaml.Build.Context, From c31bee08edb321499f7cc70edf485e6e5aadc885 Mon Sep 17 00:00:00 2001 From: Feroze Mohideen Date: Tue, 19 Sep 2023 11:22:27 -0400 Subject: [PATCH 2/2] more error handling --- internal/porter_app/v2/yaml.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/porter_app/v2/yaml.go b/internal/porter_app/v2/yaml.go index c508cbe551..57d73533fe 100644 --- a/internal/porter_app/v2/yaml.go +++ b/internal/porter_app/v2/yaml.go @@ -31,6 +31,10 @@ func AppProtoFromYaml(ctx context.Context, porterYamlBytes []byte, appName strin porterYaml.Name = appName } + if porterYaml.Name != "" && appName != "" && porterYaml.Name != appName { + return nil, nil, telemetry.Error(ctx, span, nil, "name specified in porter.yaml does not match app name") + } + appProto := &porterv1.PorterApp{ Name: porterYaml.Name, }