瀏覽代碼

Fix bug where users aren't able to change their build settings (#3212)

Feroze Mohideen 3 年之前
父節點
當前提交
d28963a490

+ 2 - 4
api/server/handlers/porter_app/create.go

@@ -376,12 +376,10 @@ func (c *CreatePorterAppHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 		if request.BuildContext != "" {
 			app.BuildContext = request.BuildContext
 		}
-		// handles deletion of builder and buildpacks
+		// handles deletion of builder,buildpacks, and dockerfile path
 		app.Builder = request.Builder
 		app.Buildpacks = request.Buildpacks
-		if request.Dockerfile != "" {
-			app.Dockerfile = request.Dockerfile
-		}
+		app.Dockerfile = request.Dockerfile
 		if request.ImageRepoURI != "" {
 			app.ImageRepoURI = request.ImageRepoURI
 		}

+ 6 - 6
dashboard/src/main/home/app-dashboard/build-settings/AdvancedBuildSettings.tsx

@@ -5,7 +5,7 @@ import Spacer from "components/porter/Spacer";
 import Input from "components/porter/Input";
 import AnimateHeight from "react-animate-height";
 import Select from "components/porter/Select";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 import BuildpackSettings from "./buildpacks/BuildpackSettings";
 import _ from "lodash";
 
@@ -13,18 +13,18 @@ interface AdvancedBuildSettingsProps {
   porterApp: PorterApp;
   updatePorterApp: (attrs: Partial<PorterApp>) => void;
   autoDetectBuildpacks: boolean;
+  buildView: BuildMethod;
+  setBuildView: (buildView: BuildMethod) => void;
 }
 
 const AdvancedBuildSettings: React.FC<AdvancedBuildSettingsProps> = ({
   porterApp,
   updatePorterApp,
   autoDetectBuildpacks,
+  buildView,
+  setBuildView,
 }) => {
   const [showSettings, setShowSettings] = useState<boolean>(false);
-  const [buildView, setBuildView] = useState<string>(
-    !_.isEmpty(porterApp.dockerfile)
-      ? "docker" : "buildpacks"
-  );
 
   return (
     <>
@@ -57,7 +57,7 @@ const AdvancedBuildSettings: React.FC<AdvancedBuildSettingsProps> = ({
               { value: "docker", label: "Docker" },
               { value: "buildpacks", label: "Buildpacks" },
             ]}
-            setValue={setBuildView}
+            setValue={(option: string) => setBuildView(option as BuildMethod)}
             label="Build method"
           />
           {buildView === "docker"

+ 7 - 1
dashboard/src/main/home/app-dashboard/build-settings/BuildSettingsTab.tsx

@@ -12,7 +12,7 @@ import { AxiosError } from "axios";
 import Button from "components/porter/Button";
 import Checkbox from "components/porter/Checkbox";
 import SharedBuildSettings from "./SharedBuildSettings";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 import _ from "lodash";
 
 type Props = {
@@ -20,6 +20,8 @@ type Props = {
   setTempPorterApp: (app: PorterApp) => void;
   updatePorterApp: (options: Partial<PorterAppOptions>) => Promise<void>;
   clearStatus: () => void;
+  buildView: BuildMethod;
+  setBuildView: (buildView: BuildMethod) => void;
 };
 
 const BuildSettingsTab: React.FC<Props> = ({
@@ -27,6 +29,8 @@ const BuildSettingsTab: React.FC<Props> = ({
   setTempPorterApp,
   clearStatus,
   updatePorterApp,
+  buildView,
+  setBuildView,
 }) => {
   const { setCurrentError, currentCluster, currentProject } = useContext(Context);
   const [redeployOnSave, setRedeployOnSave] = useState(true);
@@ -161,6 +165,8 @@ const BuildSettingsTab: React.FC<Props> = ({
         setPorterYaml={() => { }}
         autoDetectionOn={false}
         canChangeRepo={false}
+        buildView={buildView}
+        setBuildView={setBuildView}
       />
       <Spacer y={1} />
       <Checkbox

+ 3 - 0
dashboard/src/main/home/app-dashboard/build-settings/DetectDockerfileAndPorterYaml.tsx

@@ -18,12 +18,14 @@ type PropsType = {
   setPorterYaml: (yaml: string, filename: string) => void;
   porterApp: PorterApp;
   updatePorterApp: (attrs: Partial<PorterApp>) => void;
+  updateDockerfileFound: () => void;
 };
 
 const DetectDockerfileAndPorterYaml: React.FC<PropsType> = ({
   setPorterYaml,
   porterApp,
   updatePorterApp,
+  updateDockerfileFound,
 }) => {
   const [showModal, setShowModal] = useState(false);
   const [loading, setLoading] = useState(true);
@@ -68,6 +70,7 @@ const DetectDockerfileAndPorterYaml: React.FC<PropsType> = ({
 
     if (dockerFileItem) {
       updatePorterApp({ dockerfile: dockerFileItem.path });
+      updateDockerfileFound();
     }
   }, [contents]);
 

+ 8 - 1
dashboard/src/main/home/app-dashboard/build-settings/SharedBuildSettings.tsx

@@ -3,7 +3,7 @@ import Spacer from "components/porter/Spacer";
 import Text from "components/porter/Text";
 import React from "react";
 import styled from "styled-components";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 import DetectDockerfileAndPorterYaml from "./DetectDockerfileAndPorterYaml";
 import RepositorySelector from "./RepositorySelector";
 import BranchSelector from "./BranchSelector";
@@ -15,6 +15,8 @@ type Props = {
   porterApp: PorterApp;
   autoDetectionOn: boolean;
   canChangeRepo: boolean;
+  buildView: BuildMethod;
+  setBuildView: (buildView: BuildMethod) => void;
 };
 
 const SharedBuildSettings: React.FC<Props> = ({
@@ -23,6 +25,8 @@ const SharedBuildSettings: React.FC<Props> = ({
   porterApp,
   autoDetectionOn,
   canChangeRepo,
+  buildView,
+  setBuildView,
 }) => {
   return (
     <>
@@ -128,12 +132,15 @@ const SharedBuildSettings: React.FC<Props> = ({
                   setPorterYaml={setPorterYaml}
                   porterApp={porterApp}
                   updatePorterApp={updatePorterApp}
+                  updateDockerfileFound={() => setBuildView("docker")}
                 />
               )}
               <AdvancedBuildSettings
                 porterApp={porterApp}
                 updatePorterApp={updatePorterApp}
                 autoDetectBuildpacks={autoDetectionOn}
+                buildView={buildView}
+                setBuildView={setBuildView}
               />
             </>
           )}

+ 1 - 0
dashboard/src/main/home/app-dashboard/build-settings/buildpacks/BuildpackSettings.tsx

@@ -117,6 +117,7 @@ const BuildpackSettings: React.FC<{
             setAvailableBuildpacks(defaultBuilder.others);
             setError("");
           } else {
+            updatePorterApp({ builder: detectedBuilder });
             setAvailableBuildpacks(allBuildpacks.filter(bp => !porterApp.buildpacks?.includes(bp.buildpack)));
           }
         }

+ 35 - 151
dashboard/src/main/home/app-dashboard/expanded-app/ExpandedApp.tsx

@@ -48,7 +48,7 @@ import { Log } from "main/home/cluster-dashboard/expanded-chart/logs-section/use
 import Anser, { AnserJsonEntry } from "anser";
 import _ from "lodash";
 import AnimateHeight from "react-animate-height";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 
 type Props = RouteComponentProps & {};
 
@@ -97,7 +97,6 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
     false
   );
 
-  const [saveValuesStatus, setSaveValueStatus] = useState<string>("");
   const [bannerLoading, setBannerLoading] = useState<boolean>(false);
 
   const [showRevisions, setShowRevisions] = useState<boolean>(false);
@@ -120,6 +119,7 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
   const [porterApp, setPorterApp] = useState<PorterApp>();
   // this is the version of the porterApp that is being edited. on save, we set the real porter app to be this version
   const [tempPorterApp, setTempPorterApp] = useState<PorterApp>();
+  const [buildView, setBuildView] = useState<BuildMethod>("docker");
 
   const { eventId, tab } = useParams<Params>();
   const selectedTab: ValidTab = tab != null && validTabs.includes(tab) ? tab : DEFAULT_TAB;
@@ -143,12 +143,12 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
   useEffect(() => {
     const { appName } = props.match.params as any;
     if (currentCluster && appName && currentProject) {
-      getPorterApp();
+      getPorterApp({ revision: 0 });
     }
   }, [currentCluster]);
 
   // this method fetches and reconstructs the porter yaml as well as the DB info (stored in PorterApp)
-  const getPorterApp = async () => {
+  const getPorterApp = async ({ revision }: { revision: number }) => {
     setBannerLoading(true);
     setIsLoading(true);
     const { appName } = props.match.params as any;
@@ -173,14 +173,14 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
           namespace: `porter-stack-${appName}`,
           cluster_id: currentCluster.id,
           name: appName,
-          revision: 0,
+          revision: revision,
         }
       );
 
-      let releaseChartData;
-      // get the release chart
+      let preDeployChartData;
+      // get the pre-deploy chart
       try {
-        releaseChartData = await api.getChart(
+        preDeployChartData = await api.getChart(
           "<token>",
           {},
           {
@@ -188,6 +188,7 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
             namespace: `porter-stack-${appName}`,
             cluster_id: currentCluster.id,
             name: `${appName}-r`,
+            // this is always latest because we do not tie the pre-deploy chart to the umbrella chart
             revision: 0,
           }
         );
@@ -200,7 +201,7 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
       const newAppData = {
         app: resPorterApp?.data,
         chart: resChartData?.data,
-        releaseChart: releaseChartData?.data,
+        releaseChart: preDeployChartData?.data,
       };
       const porterJson = await fetchPorterYamlContent(
         resPorterApp?.data?.porter_yaml_path ?? "porter.yaml",
@@ -213,10 +214,11 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
       const parsedPorterApp = { ...resPorterApp?.data, buildpacks: newAppData.app.buildpacks?.split(",") ?? [] };
       setPorterApp(parsedPorterApp);
       setTempPorterApp(parsedPorterApp);
+      setBuildView(!_.isEmpty(parsedPorterApp.dockerfile) ? "docker" : "buildpacks")
 
       const [newServices, newEnvVars] = updateServicesAndEnvVariables(
         resChartData?.data,
-        releaseChartData?.data,
+        preDeployChartData?.data,
         porterJson,
       );
       const finalPorterYaml = createFinalPorterYaml(
@@ -333,19 +335,27 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
         );
         const yamlString = yaml.dump(finalPorterYaml);
         const base64Encoded = btoa(yamlString);
+
+        const updatedPorterApp = {
+          porter_yaml: base64Encoded,
+          override_release: true,
+          ...PorterApp.empty(),
+          build_context: tempPorterApp.build_context,
+          repo_name: tempPorterApp.repo_name,
+          git_branch: tempPorterApp.git_branch,
+          buildpacks: "",
+          ...options,
+        }
+        if (buildView === "docker") {
+          updatedPorterApp.dockerfile = tempPorterApp.dockerfile;
+        } else {
+          updatedPorterApp.builder = tempPorterApp.builder;
+          updatedPorterApp.buildpacks = tempPorterApp.buildpacks.join(",");
+        }
+
         await api.createPorterApp(
           "<token>",
-          {
-            porter_yaml: base64Encoded,
-            repo_name: tempPorterApp.repo_name,
-            git_branch: tempPorterApp.git_branch,
-            build_context: tempPorterApp.build_context,
-            builder: tempPorterApp.builder,
-            buildpacks: tempPorterApp.buildpacks.join(","),
-            dockerfile: tempPorterApp.dockerfile,
-            ...options,
-            override_release: true,
-          },
+          updatedPorterApp,
           {
             cluster_id: currentCluster.id,
             project_id: currentProject.id,
@@ -528,137 +538,10 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
     return [newServices, envVars];
   };
 
-  const getChartData = async (chart: ChartType, isCurrent?: boolean) => {
-    setButtonStatus("");
-    setIsLoading(true);
-    try {
-      const res = await api.getChart(
-        "<token>",
-        {},
-        {
-          name: chart.name,
-          namespace: chart.namespace,
-          cluster_id: currentCluster.id,
-          revision: chart.version,
-          id: currentProject.id,
-        }
-      );
-
-      const updatedChart = res.data;
-
-      if (appData != null && updatedChart != null) {
-        setAppData({ ...appData, chart: updatedChart });
-      }
-
-      // let releaseChartData;
-      // // get the release chart
-      // try {
-      //   releaseChartData = await api.getChart(
-      //     "<token>",
-      //     {},
-      //     {
-      //       id: currentProject.id,
-      //       namespace: `porter-stack-${chart.name}`,
-      //       cluster_id: currentCluster.id,
-      //       name: `${chart.name}-r`,
-      //       revision: 0,
-      //     }
-      //   );
-      // } catch (err) {
-      //   // do nothing, unable to find release chart
-      //   // console.log(err);
-      // }
-
-      // const releaseChart = releaseChartData?.data;
-
-      // if (appData != null && updatedChart != null) {
-      //   if (releaseChart != null) {
-      //     setAppData({ ...appData, chart: updatedChart, releaseChart });
-      //   } else {
-      //     setAppData({ ...appData, chart: updatedChart });
-      //   }
-      // }
-
-      const [newServices, newEnvVars] = updateServicesAndEnvVariables(
-        updatedChart,
-        appData.releaseChart,
-        porterJson,
-        appData.app.builder != null && appData.app.builder.includes("heroku")
-      );
-
-      if (isCurrent) {
-        setShowUnsavedChangesBanner(false);
-      } else {
-        onAppUpdate(newServices, newEnvVars);
-      }
-    } catch (err) {
-      console.log(err);
-    } finally {
-      setIsLoading(false);
-    }
-
-  };
-
   const setRevision = (chart: ChartType, isCurrent?: boolean) => {
-    getChartData(chart, isCurrent);
+    getPorterApp({ revision: chart.version });
   };
 
-  const appUpgradeVersion = useCallback(
-    async (version: string, cb: () => void) => {
-      // convert current values to yaml
-      const values = appData.chart.config;
-
-      const valuesYaml = yaml.dump({
-        ...values,
-      });
-
-      setSaveValueStatus("loading");
-      getChartData(appData.chart);
-
-      try {
-        await api.upgradeChartValues(
-          "<token>",
-          {
-            values: valuesYaml,
-            version: version,
-            latest_revision: appData.chart.version,
-          },
-          {
-            id: currentProject.id,
-            namespace: appData.chart.namespace,
-            name: appData.chart.name,
-            cluster_id: currentCluster.id,
-          }
-        );
-        setSaveValueStatus("successful");
-        setForceRefreshRevisions(true);
-
-        window.analytics?.track("Chart Upgraded", {
-          chart: appData.chart.name,
-          values: valuesYaml,
-        });
-
-        cb && cb();
-      } catch (err) {
-        const parsedErr = err?.response?.data?.error;
-
-        if (parsedErr) {
-          err = parsedErr;
-        }
-
-        setSaveValueStatus(err);
-        setCurrentError(parsedErr);
-
-        window.analytics?.track("Failed to Upgrade Chart", {
-          chart: appData.chart.name,
-          values: valuesYaml,
-          error: err,
-        });
-      }
-    },
-    [appData?.chart]
-  );
-
   const getReadableDate = (s: string) => {
     const ts = new Date(s);
     const date = ts.toLocaleDateString();
@@ -762,6 +645,8 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
             clearStatus={() => setButtonStatus("")}
             updatePorterApp={updatePorterApp}
             setShowUnsavedChangesBanner={setShowUnsavedChangesBanner}
+            buildView={buildView}
+            setBuildView={setBuildView}
           />
         );
       case "settings":
@@ -824,7 +709,7 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
 
   return (
     <>
-      {isLoading && appData == null && <Loading />}
+      {isLoading && <Loading />}
       {!isLoading && appData == null && (
         <Placeholder>
           <Container row>
@@ -977,7 +862,6 @@ const ExpandedApp: React.FC<Props> = ({ ...props }) => {
                       appData.chart.chart.metadata.version
                     }
                     latestVersion={appData.chart.latest_version}
-                    upgradeVersion={appUpgradeVersion}
                   />
                   <DarkMatter antiHeight="-18px" />
                 </>

+ 27 - 16
dashboard/src/main/home/app-dashboard/new-app-flow/NewAppFlow.tsx

@@ -27,7 +27,7 @@ import { PorterJson, PorterYamlSchema, createFinalPorterYaml } from "./schema";
 import { Service } from "./serviceTypes";
 import GithubConnectModal from "./GithubConnectModal";
 import Link from "components/porter/Link";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 
 type Props = RouteComponentProps & {};
 
@@ -95,6 +95,8 @@ const NewAppFlow: React.FC<Props> = ({ ...props }) => {
 
   const [porterJsonWithPath, setPorterJsonWithPath] = useState<PorterJsonWithPath | undefined>(undefined);
   const [detected, setDetected] = useState<Detected | undefined>(undefined);
+  const [buildView, setBuildView] = useState<BuildMethod>("buildpacks");
+
   const handleSetAccessData = (data: GithubAppAccessData) => {
     setAccessData(data);
     setShowGithubConnectModal(
@@ -283,23 +285,30 @@ const NewAppFlow: React.FC<Props> = ({ ...props }) => {
         };
       }
 
+      const porterAppRequest = {
+        porter_yaml: base64Encoded,
+        override_release: true,
+        image_info: imageInfo,
+        ...PorterApp.empty(),
+        buildpacks: "",
+        // for some reason I couldn't get the path to update the porterApp object correctly here so I just grouped it with the porter json :/
+        porter_yaml_path: porterJsonWithPath?.porterYamlPath,
+        repo_name: porterApp.repo_name,
+        git_branch: porterApp.git_branch,
+        git_repo_id: porterApp.git_repo_id,
+        build_context: porterApp.build_context,
+        image_repo_uri: porterApp.image_repo_uri,
+      }
+      if (buildView === "docker") {
+        porterAppRequest.dockerfile = porterApp.dockerfile;
+      } else {
+        porterAppRequest.builder = porterApp.builder;
+        porterAppRequest.buildpacks = porterApp.buildpacks.join(",");
+      }
+
       await api.createPorterApp(
         "<token>",
-        {
-          repo_name: porterApp.repo_name,
-          git_branch: porterApp.git_branch,
-          git_repo_id: porterApp.git_repo_id,
-          build_context: porterApp.build_context,
-          builder: !_.isEmpty(porterApp.dockerfile) ? "" : porterApp.builder,
-          buildpacks: !_.isEmpty(porterApp.dockerfile) ? "" : porterApp.buildpacks.join(","),
-          dockerfile: porterApp.dockerfile,
-          image_repo_uri: porterApp.image_repo_uri,
-          porter_yaml: base64Encoded,
-          override_release: true,
-          image_info: imageInfo,
-          // for some reason I couldn't get the path to update the porterApp object correctly here so I just grouped it with the porter json :/
-          porter_yaml_path: porterJsonWithPath?.porterYamlPath,
-        },
+        porterAppRequest,
         {
           cluster_id: currentCluster.id,
           project_id: currentProject.id,
@@ -419,6 +428,8 @@ const NewAppFlow: React.FC<Props> = ({ ...props }) => {
                   }}
                   imageTag={imageTag}
                   setImageTag={setImageTag}
+                  buildView={buildView}
+                  setBuildView={setBuildView}
                 />
               </>,
               <>

+ 7 - 1
dashboard/src/main/home/app-dashboard/new-app-flow/SourceSettings.tsx

@@ -8,7 +8,7 @@ import { pushFiltered } from "shared/routing";
 import ImageSelector from "components/image-selector/ImageSelector";
 import SharedBuildSettings from "../build-settings/SharedBuildSettings";
 import Link from "components/porter/Link";
-import { PorterApp } from "../types/porterApp";
+import { BuildMethod, PorterApp } from "../types/porterApp";
 
 type Props = RouteComponentProps & {
   source: SourceType | undefined;
@@ -19,6 +19,8 @@ type Props = RouteComponentProps & {
   setPorterYaml: (yaml: string, filename: string) => void;
   porterApp: PorterApp;
   setPorterApp: (x: PorterApp) => void;
+  buildView: BuildMethod;
+  setBuildView: (buildView: BuildMethod) => void;
 };
 
 const SourceSettings: React.FC<Props> = ({
@@ -30,6 +32,8 @@ const SourceSettings: React.FC<Props> = ({
   setPorterYaml,
   porterApp,
   setPorterApp,
+  buildView,
+  setBuildView,
   location,
   history,
 }) => {
@@ -44,6 +48,8 @@ const SourceSettings: React.FC<Props> = ({
             updatePorterApp={(attrs: Partial<PorterApp>) => setPorterApp(PorterApp.setAttributes(porterApp, attrs))}
             autoDetectionOn={true}
             canChangeRepo={true}
+            buildView={buildView}
+            setBuildView={setBuildView}
           />
         ) : (
           <StyledSourceBox>

+ 3 - 1
dashboard/src/main/home/app-dashboard/types/porterApp.ts

@@ -38,4 +38,6 @@ export const PorterApp = {
         ...app,
         ...values,
     }),
-}
+}
+
+export type BuildMethod = "docker" | "buildpacks";