Skip to content

Commit

Permalink
Addressed comment and kept support for admin cred
Browse files Browse the repository at this point in the history
  • Loading branch information
@harryli0108
harryli0108 committed Dec 4, 2023
1 parent c9cf98d commit 045cbde
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 22 deletions.
15 changes: 10 additions & 5 deletions azurecontainerapps.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export class azurecontainerapps {
private static ingressEnabled: boolean;

// Container Registry properties
private static adminCredentialsProvided: boolean;
private static registryUsername: string;
private static registryPassword: string;
private static registryUrl: string;
Expand All @@ -103,7 +104,7 @@ export class azurecontainerapps {
private static imageToBuild: string;
private static ingress: string;
private static targetPort: string;
private static shouldUseUpdateCommand: boolean;
private static noIngressUpdate: boolean;
private static useInternalRegistry: boolean;
private static shouldCreateOrUpdateContainerAppWithUp: boolean;

Expand Down Expand Up @@ -502,13 +503,14 @@ export class azurecontainerapps {

// If both ingress and target port were not provided for an existing Container App, or if ingress is to be disabled,
// use the 'update' command, otherwise we should use the 'up' command that performs a PATCH operation on the ingress properties.
this.shouldUseUpdateCommand = this.containerAppExists &&
this.noIngressUpdate = this.containerAppExists &&
this.util.isNullOrEmpty(this.targetPort) &&
(this.util.isNullOrEmpty(this.ingress) || this.ingress == 'disabled');

// Pass the Container Registry credentials when creating a Container App or updating a Container App via the 'up' command
if (!this.util.isNullOrEmpty(this.registryUrl) && !this.util.isNullOrEmpty(this.registryUsername) && !this.util.isNullOrEmpty(this.registryPassword) &&
(!this.containerAppExists || (this.containerAppExists && !this.shouldUseUpdateCommand))) {
(!this.containerAppExists || (this.containerAppExists && !this.noIngressUpdate))) {
this.adminCredentialsProvided = true;
this.commandLineArgs.push(
`--registry-server ${this.registryUrl}`,
`--registry-username ${this.registryUsername}`,
Expand Down Expand Up @@ -555,7 +557,7 @@ export class azurecontainerapps {
if (!this.util.isNullOrEmpty(environmentVariables)) {
// The --replace-env-vars flag is only used for the 'update' command,
// otherwise --env-vars is used for 'create' and 'up'
if (this.shouldUseUpdateCommand) {
if (this.noIngressUpdate) {
this.commandLineArgs.push(`--replace-env-vars ${environmentVariables}`);
} else {
this.commandLineArgs.push(`--env-vars ${environmentVariables}`);
Expand Down Expand Up @@ -597,7 +599,7 @@ export class azurecontainerapps {
return;
}

if (this.shouldUseUpdateCommand && !this.shouldCreateOrUpdateContainerAppWithUp) {
if (this.noIngressUpdate && !this.shouldCreateOrUpdateContainerAppWithUp) {
// Update the Container Registry details on the existing Container App, if provided as an input
if (!this.util.isNullOrEmpty(this.registryUrl) && !this.util.isNullOrEmpty(this.registryUsername) && !this.util.isNullOrEmpty(this.registryPassword)) {
await this.appHelper.updateContainerAppRegistryDetails(this.containerAppName, this.resourceGroup, this.registryUrl, this.registryUsername, this.registryPassword);
Expand All @@ -607,6 +609,9 @@ export class azurecontainerapps {
await this.appHelper.updateContainerApp(this.containerAppName, this.resourceGroup, this.commandLineArgs);
} else if (this.shouldCreateOrUpdateContainerAppWithUp) {
await this.appHelper.createOrUpdateContainerAppWithUp(this.containerAppName, this.resourceGroup, this.commandLineArgs);
} else if (this.adminCredentialsProvided && !this.noIngressUpdate) {
// Update the Container App with `up` command when admin credentials are provided and ingress is manually provided.
await this.appHelper.updateContainerAppWithUp(this.containerAppName, this.resourceGroup, this.commandLineArgs, this.ingress, this.targetPort);
} else {
// Update the Container App using the 'containerapp update' and 'ingress update' commands
await this.appHelper.updateContainerApp(this.containerAppName, this.resourceGroup, this.commandLineArgs)
Expand Down
40 changes: 24 additions & 16 deletions dist/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,12 +603,13 @@ var azurecontainerapps = /** @class */ (function () {
this.targetPort = this.toolHelper.getInput('targetPort', false);
// If both ingress and target port were not provided for an existing Container App, or if ingress is to be disabled,
// use the 'update' command, otherwise we should use the 'up' command that performs a PATCH operation on the ingress properties.
this.shouldUseUpdateCommand = this.containerAppExists &&
this.noIngressUpdate = this.containerAppExists &&
this.util.isNullOrEmpty(this.targetPort) &&
(this.util.isNullOrEmpty(this.ingress) || this.ingress == 'disabled');
// Pass the Container Registry credentials when creating a Container App or updating a Container App via the 'up' command
if (!this.util.isNullOrEmpty(this.registryUrl) && !this.util.isNullOrEmpty(this.registryUsername) && !this.util.isNullOrEmpty(this.registryPassword) &&
(!this.containerAppExists || (this.containerAppExists && !this.shouldUseUpdateCommand))) {
(!this.containerAppExists || (this.containerAppExists && !this.noIngressUpdate))) {
this.adminCredentialsProvided = true;
this.commandLineArgs.push("--registry-server ".concat(this.registryUrl), "--registry-username ".concat(this.registryUsername), "--registry-password ".concat(this.registryPassword));
}
// Determine default values only for the 'create' scenario to avoid overriding existing values for the 'update' scenario
Expand Down Expand Up @@ -644,7 +645,7 @@ var azurecontainerapps = /** @class */ (function () {
if (!this.util.isNullOrEmpty(environmentVariables)) {
// The --replace-env-vars flag is only used for the 'update' command,
// otherwise --env-vars is used for 'create' and 'up'
if (this.shouldUseUpdateCommand) {
if (this.noIngressUpdate) {
this.commandLineArgs.push("--replace-env-vars ".concat(environmentVariables));
}
else {
Expand Down Expand Up @@ -699,7 +700,7 @@ var azurecontainerapps = /** @class */ (function () {
_a.sent();
return [2 /*return*/];
case 9:
if (!(this.shouldUseUpdateCommand && !this.shouldCreateOrUpdateContainerAppWithUp)) return [3 /*break*/, 13];
if (!(this.noIngressUpdate && !this.shouldCreateOrUpdateContainerAppWithUp)) return [3 /*break*/, 13];
if (!(!this.util.isNullOrEmpty(this.registryUrl) && !this.util.isNullOrEmpty(this.registryUsername) && !this.util.isNullOrEmpty(this.registryPassword))) return [3 /*break*/, 11];
return [4 /*yield*/, this.appHelper.updateContainerAppRegistryDetails(this.containerAppName, this.resourceGroup, this.registryUrl, this.registryUsername, this.registryPassword)];
case 10:
Expand All @@ -711,30 +712,38 @@ var azurecontainerapps = /** @class */ (function () {
case 12:
// Update the Container App using the 'update' command
_a.sent();
return [3 /*break*/, 18];
return [3 /*break*/, 20];
case 13:
if (!this.shouldCreateOrUpdateContainerAppWithUp) return [3 /*break*/, 15];
return [4 /*yield*/, this.appHelper.createOrUpdateContainerAppWithUp(this.containerAppName, this.resourceGroup, this.commandLineArgs)];
case 14:
_a.sent();
return [3 /*break*/, 18];
case 15:
return [3 /*break*/, 20];
case 15:
if (!(this.adminCredentialsProvided && !this.noIngressUpdate)) return [3 /*break*/, 17];
// Update the Container App with `up` command when admin credentials are provided and ingress is manually provided.
return [4 /*yield*/, this.appHelper.updateContainerAppWithUp(this.containerAppName, this.resourceGroup, this.commandLineArgs, this.ingress, this.targetPort)];
case 16:
// Update the Container App with `up` command when admin credentials are provided and ingress is manually provided.
_a.sent();
return [3 /*break*/, 20];
case 17:
// Update the Container App using the 'containerapp update' and 'ingress update' commands
return [4 /*yield*/, this.appHelper.updateContainerApp(this.containerAppName, this.resourceGroup, this.commandLineArgs)];
case 16:
case 18:
// Update the Container App using the 'containerapp update' and 'ingress update' commands
_a.sent();
return [4 /*yield*/, this.appHelper.updateContainerAppIngress(this.containerAppName, this.resourceGroup, this.ingress, this.targetPort)];
case 17:
_a.sent();
_a.label = 18;
case 18:
if (!(this.ingress == 'disabled')) return [3 /*break*/, 20];
return [4 /*yield*/, this.appHelper.disableContainerAppIngress(this.containerAppName, this.resourceGroup)];
case 19:
_a.sent();
_a.label = 20;
case 20: return [2 /*return*/];
case 20:
if (!(this.ingress == 'disabled')) return [3 /*break*/, 22];
return [4 /*yield*/, this.appHelper.disableContainerAppIngress(this.containerAppName, this.resourceGroup)];
case 21:
_a.sent();
_a.label = 22;
case 22: return [2 /*return*/];
}
});
});
Expand Down Expand Up @@ -4935,7 +4944,6 @@ var ContainerAppHelper = /** @class */ (function () {
* Update container app with update and ingress update to avoid failure of acr authentication.
* @param containerAppName - the name of the existing Container App
* @param resourceGroup - the resource group that the existing Container App is found in
* @param optionalCmdArgs - a set of optional command line arguments
* @param ingress - the ingress that the Container App will be exposed on
* @param targetPort - the target port that the Container App will be exposed on
*/
Expand Down
1 change: 0 additions & 1 deletion src/ContainerAppHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ export class ContainerAppHelper {
* Update container app with update and ingress update to avoid failure of acr authentication.
* @param containerAppName - the name of the existing Container App
* @param resourceGroup - the resource group that the existing Container App is found in
* @param optionalCmdArgs - a set of optional command line arguments
* @param ingress - the ingress that the Container App will be exposed on
* @param targetPort - the target port that the Container App will be exposed on
*/
Expand Down

0 comments on commit 045cbde

Please sign in to comment.