Skip to content
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.

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

[BUG] Azure Container App SDK - Patch (Update) operation erroneously sends empty secret values, makes update via SDK impossible #35004

Closed
derSchtefan opened this issue Mar 20, 2023 · 6 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Container Service customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Mgmt This issue is related to a management-plane library.

Comments

@derSchtefan
Copy link

Library name and version

Azure.ResourceManager.AppContainers 1.0.2

Describe the bug

When an Azure Container App contains secrets (in our case: the container registry password),
when using the SDK to update the container image tag in the template
the SDK tries to send a PATCH request containing empty secrets in the configuration property, which will trigger an error like:

Invalid Request: Container app secret(s) with name(s) 'secret1, secret2' are invalid: value or keyVaultUrl and identity should be provided.

secret1 and secret2 are secrets defined on the app

Using the REST api manually works as expected:

PATCH https://management.azure.com/subscriptions/x/resourceGroups/y/providers/Microsoft.App/containerApps/yadayada?api-version=2022-03-01
{
  "properties": {
    "template": {
      "containers": [
        {
          "image": "yadayada.io/bla:1.2.0",
          "name": "my-container"
        }
      ]
    }
  }
}

The SDK should not send back the properties.configuration.secrets array in a PATCH, unless the secrets have been changed (because the secrets array after a GET does not contain all information).

Ideally the SDK would not touch properties.configuration at all, if only the template or image tags in it are touched.

Expected behavior

The SDK does not send the properties.configuration.secrets array, and the request is accepted with HTTP 202 (like when doing it manually via REST)

Actual behavior

The SDK sends an invalid (half-filled) secrets array, leading to Invalid Request: Container app secret(s) with name(s) 'secret1, secret2' are invalid: value or keyVaultUrl and identity should be provided.

Reproduction Steps

Use Azure.ResourceManager.AppContainers v1.0.2
Use this code:

var containerApp = await resourceGroup.GetContainerAppAsync(request.ResourceName);
var containerAppData = containerApp.Value.Data;
var container = containerAppData.Template.Containers.Single();

var originalContainerImage = container.Image;
container.Image = "yadayada.io/bla:1.2.0";

//containerAppData.Configuration.Secrets contains secrets, but without the value

var updateResult = await containerApp.Value.UpdateAsync(WaitUntil.Completed, containerAppData);

Environment

Azure Container Apps
Azure.ResourceManager.AppContainers 1.0.2

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Mar 20, 2023
@jsquire jsquire added Mgmt This issue is related to a management-plane library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Container Service labels Mar 20, 2023
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 20, 2023
@jsquire
Copy link
Member

jsquire commented Mar 20, 2023

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@Guillaume-R-FLOSS
Copy link

We are facing the same problem. Although we could re-wrap the Rest API our self it would make this SDK pointless. It would be very appreciated if the SDK could offer the same capabilities as the Rest API.

@ArthurMa1978 ArthurMa1978 added bug This issue requires a change to an existing behavior in the product in order to be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 22, 2023
@groogiam
Copy link

Is there any update on this? This is pretty significant defect with no update in several months.

@HarveyLink
Copy link
Member

Hi @derSchtefan @groogiam , Thank you for using Azure SDK for .NET.
The response value(in this case, secret related value) comes from service side, the SDK only convert it to readable values.
As your sample suggest, you were using the whole response data. So when you only update template parts of this data, other values remains untouched, it will cause service error(since secret value is not complete).
If you would like to do a PATCH update, it's more common to only add what you need to the data class, without any onther properties. I suggest you could use it like:

    var apps = resourceGroup.GetContainerApps();
    var resource = (await apps.GetAsync("test")).Value;
    var template = resource.Data.Template;
    template.Containers.Single().Image = "test.azurecr.io/hello-world:t2";
    var data = new ContainerAppData(AzureLocation.EastUS)
    {
        Template = template
    };
    resource = (await resource.UpdateAsync(WaitUntil.Completed, data)).Value;

@HarveyLink HarveyLink added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Jan 15, 2024
@github-actions github-actions bot removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 15, 2024
Copy link

Hi @derSchtefan. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

Copy link

Hi @derSchtefan, since you haven’t asked that we /unresolve the issue, we’ll close this out. If you believe further discussion is needed, please add a comment /unresolve to reopen the issue.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Container Service customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

No branches or pull requests

6 participants