Misc. improvements to ingress logic (#783)

Author: MicroFish91

src/commands/image/imageSource/ContainerAppUpdateStep.ts

@@ -3,22 +3,41 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { type Ingress } from "@azure/arm-appcontainers";
 import { AzureWizardExecuteStep, GenericParentTreeItem, GenericTreeItem, activityFailContext, activityFailIcon, activitySuccessContext, activitySuccessIcon, createUniversallyUniqueContextValue, nonNullProp, type ExecuteActivityOutput } from "@microsoft/vscode-azext-utils";
 import { type Progress } from "vscode";
 import { ext } from "../../../extensionVariables";
 import { getContainerEnvelopeWithSecrets, type ContainerAppModel } from "../../../tree/ContainerAppItem";
 import { localize } from "../../../utils/localize";
+import { type IngressContext } from "../../ingress/IngressContext";
+import { enabledIngressDefaults } from "../../ingress/enableIngress/EnableIngressStep";
 import { updateContainerApp } from "../../updateContainerApp";
 import { type ImageSourceContext } from "./ImageSourceContext";
 import { getContainerNameForImage } from "./containerRegistry/getContainerNameForImage";
 
-export class ContainerAppUpdateStep<T extends ImageSourceContext> extends AzureWizardExecuteStep<T> {
+export class ContainerAppUpdateStep<T extends ImageSourceContext & IngressContext> extends AzureWizardExecuteStep<T> {
     public priority: number = 680;
 
     public async execute(context: T, progress: Progress<{ message?: string | undefined; increment?: number | undefined }>): Promise<void> {
         const containerApp: ContainerAppModel = nonNullProp(context, 'containerApp');
         const containerAppEnvelope = await getContainerEnvelopeWithSecrets(context, context.subscription, containerApp);
 
+        let ingress: Ingress | undefined;
+        if (context.enableIngress) {
+            ingress = {
+                ...enabledIngressDefaults,
+                ...containerAppEnvelope.configuration.ingress ?? {}, // Overwrite any default settings if we already have previous configurations set
+                external: context.enableExternal ?? containerAppEnvelope.configuration.ingress?.external,
+                targetPort: context.targetPort ?? containerAppEnvelope.configuration.ingress?.targetPort,
+            };
+        } else if (context.enableIngress === false) {
+            ingress = undefined;
+        } else {
+            // If enableIngress is not set, just default to the previous settings if they exist
+            ingress = containerAppEnvelope.configuration.ingress;
+        }
+
+        containerAppEnvelope.configuration.ingress = ingress;
         containerAppEnvelope.configuration.secrets = context.secrets;
         containerAppEnvelope.configuration.registries = context.registryCredentials;
 

src/commands/ingress/IngressPromptStep.ts

@@ -59,28 +59,31 @@ export async function tryConfigureIngressUsingDockerfile(context: IngressContext
         return;
     }
 
-    if (!context.dockerfileExposePorts) {
-        context.enableIngress = false;
-        context.enableExternal = false;
-    } else if (context.dockerfileExposePorts) {
+    if (context.dockerfileExposePorts) {
         context.enableIngress = true;
         context.enableExternal = true;
         context.targetPort = getDefaultPort(context);
+    } else {
+        context.enableIngress = false;
+        context.enableExternal = false;
     }
 
-    // If a container app already exists, activity children will be added automatically in later execute steps
-    if (!context.containerApp) {
-        context.activityChildren?.push(
-            new GenericTreeItem(undefined, {
-                contextValue: createUniversallyUniqueContextValue(['ingressPromptStepSuccessItem', activitySuccessContext]),
-                label: context.enableIngress ?
-                    localize('ingressEnableLabel', 'Enable ingress on port {0} (from Dockerfile configuration)', context.targetPort) :
-                    localize('ingressDisableLabel', 'Disable ingress (from Dockerfile configuration)'),
-                iconPath: activitySuccessIcon
-            })
-        );
+    const currentExternalEnabled: boolean | undefined = context.containerApp?.configuration?.ingress?.external;
+    const currentTargetPort: number | undefined = context.containerApp?.configuration?.ingress?.targetPort;
+    if (currentExternalEnabled === context.enableExternal && currentTargetPort === context.targetPort) {
+        return;
     }
 
+    context.activityChildren?.push(
+        new GenericTreeItem(undefined, {
+            contextValue: createUniversallyUniqueContextValue(['ingressPromptStepSuccessItem', activitySuccessContext]),
+            label: context.enableIngress ?
+                localize('ingressEnableLabel', 'Enable ingress on port {0} (from Dockerfile configuration)', context.targetPort) :
+                localize('ingressDisableLabel', 'Disable ingress (from Dockerfile configuration)'),
+            iconPath: activitySuccessIcon
+        })
+    );
+
     ext.outputChannel.appendLog(context.enableIngress ?
         localize('ingressEnabledLabel', 'Detected ingress on port {0} using Dockerfile configuration.', context.targetPort) :
         localize('ingressDisabledLabel', 'Detected no ingress using Dockerfile configuration.')

src/commands/ingress/enableIngress/EnableIngressStep.ts

@@ -10,6 +10,17 @@ import { localize } from "../../../utils/localize";
 import { updateContainerApp } from "../../updateContainerApp";
 import { type IngressBaseContext } from "../IngressContext";
 
+export const enabledIngressDefaults = {
+    transport: 'auto',
+    allowInsecure: false,
+    traffic: [
+        {
+            weight: 100,
+            latestRevision: true
+        }
+    ],
+};
+
 export class EnableIngressStep extends AzureWizardExecuteStep<IngressBaseContext> {
     public priority: number = 750;
 
@@ -18,19 +29,13 @@ export class EnableIngressStep extends AzureWizardExecuteStep<IngressBaseContext
 
         const containerApp = nonNullProp(context, 'containerApp');
         const ingress: Ingress = {
+            ...enabledIngressDefaults,
+            ...containerApp.configuration?.ingress ?? {},
             targetPort: context.targetPort,
             external: context.enableExternal,
-            transport: 'auto',
-            allowInsecure: false,
-            traffic: [
-                {
-                    weight: 100,
-                    latestRevision: true
-                }
-            ],
         }
 
-        await updateContainerApp(context, context.subscription, containerApp, { configuration: { ingress: ingress as Ingress | undefined } });
+        context.containerApp = await updateContainerApp(context, context.subscription, containerApp, { configuration: { ingress: ingress as Ingress | undefined } });
     }
 
     public shouldExecute(context: IngressBaseContext): boolean {