RegisterTaskDefinitionAsync cannot be used to register Fargate Tasks due to invalid Property Types

[chaseaucoin]

Description

RegisterTaskDefinitionAsync can not be used to register Fargate tasks due to incorrect strongly typed mappings. For the container definition Fargate accepts null entries for many of the options, including most importantly for my needs memory and memory reservation. This gets set at the task group level and is not necessary at the individual container level. As it stands if I take the definition of a task that was registered using the AWS UI, I cannot modify it using the SDK due to this limitation.

Reproduction Steps

Inspecting the request on the AWS UI at https://console.aws.amazon.com/ecs/home?region=us-east-1#/taskDefinitions I can see the following JSON. (sensitive information in curly braces ommited)

{
    "ipcMode": null,
    "executionRoleArn": "arn:aws:iam::{someID}:role/{someRole}",
    "containerDefinitions": [{
            "dnsSearchDomains": null,
            "environmentFiles": null,
            "logConfiguration": {
                "logDriver": "awslogs",
                "secretOptions": null,
                "options": {
                    "awslogs-group": "{someLogGroup}",
                    "awslogs-region": "us-east-1",
                    "awslogs-stream-prefix": "ecs"
                }
            },
            "entryPoint": null,
            "portMappings": [{
                    "protocol": "tcp",
                    "containerPort": {somePort}
                }
            ],
            "command": null,
            "linuxParameters": null,
            "cpu": 0,
            "environment": [],
            "resourceRequirements": null,
            "ulimits": null,
            "dnsServers": null,
            "mountPoints": [],
            "workingDirectory": null,
            "secrets": null,
            "dockerSecurityOptions": null,
            "memory": null,
            "memoryReservation": null,
            "volumesFrom": [],
            "stopTimeout": null,
            "image": "{someImage}",
            "startTimeout": null,
            "firelensConfiguration": null,
            "dependsOn": null,
            "disableNetworking": null,
            "interactive": null,
            "healthCheck": null,
            "essential": true,
            "links": null,
            "hostname": null,
            "extraHosts": null,
            "pseudoTerminal": null,
            "user": null,
            "readonlyRootFilesystem": null,
            "dockerLabels": null,
            "systemControls": null,
            "privileged": null,
            "name": "service"
        }
    ],
    "memory": "2048",
    "taskRoleArn": "arn:aws:iam::{someID}:role/{someRole}",
    "family": "internal-services-template",
    "pidMode": null,
    "requiresCompatibilities": ["FARGATE"],
    "networkMode": "awsvpc",
    "cpu": "256",
    "inferenceAccelerators": [],
    "proxyConfiguration": null,
    "volumes": []
}

If I call DescribeTaskDefinitionAsync on this task definition all of the null values will be replaced with default int or bool values. 0 or false. If I then try to use those container definitions to call RegisterTaskDefinitionAsync it will fail due to invalid values on several of the properties.

Logs

Environment

• SDK Version: All
• Package Version: All
• OS Info: All
• Build Environment All
• Targeted .NET Platform:

Resolution

• 👋 I can/would-like-to implement a fix for this problem myself

move all bool, and int properties to their nullable equivalents bool? and int?
https://github.com/aws/aws-sdk-net/blob/master/sdk/src/Services/ECS/Generated/Model/ContainerDefinition.cs

---

This is a 🐛 bug-report

[ashishdhingra]

The class ContainerDefinition already defines nullable types for int, bool, etc. However, executing the statement AmazonECSClient.DescribeTaskDefinitionAsync() returns the default values 0 (for int?) and false (for bool?) for nullable types for below JSON:

"containerDefinitions": [
    {
      "dnsSearchDomains": null,
      "environmentFiles": null,
      "logConfiguration": {
        "logDriver": "awslogs",
        "secretOptions": null,
        "options": {
          "awslogs-group": "/ecs/test",
          "awslogs-region": "us-east-1",
          "awslogs-stream-prefix": "ecs"
        }
      },
      "entryPoint": null,
      "portMappings": [],
      "command": null,
      "linuxParameters": null,
      "cpu": 0,
      "environment": [],
      "resourceRequirements": null,
      "ulimits": null,
      "dnsServers": null,
      "mountPoints": [],
      "workingDirectory": null,
      "secrets": null,
      "dockerSecurityOptions": null,
      "memory": null,
      "memoryReservation": null,
      "volumesFrom": [],
      "stopTimeout": null,
      "image": "test",
      "startTimeout": null,
      "firelensConfiguration": null,
      "dependsOn": null,
      "disableNetworking": null,
      "interactive": null,
      "healthCheck": null,
      "essential": true,
      "links": null,
      "hostname": null,
      "extraHosts": null,
      "pseudoTerminal": null,
      "user": null,
      "readonlyRootFilesystem": null,
      "dockerLabels": null,
      "systemControls": null,
      "privileged": null,
      "name": "testcontainer"
    }
  ]

Looks like the property definitions for the private nullable fields are declared with primitive non-nullable return types. Just to mention, it appears that the code is generated from service model JSON file.

[chaseaucoin]

"The class ContainerDefinition already defines nullable types for int, bool, etc."

@ashishdhingra no it doesn't. The private fields are nullable but the public members are not and that is what gets serialized.

Example:

Line 815
public int Memory
{
get { return this._memory.GetValueOrDefault(); }
set { this._memory = value; }
}

[ashishdhingra]

@chaseaucoin Thanks for pointing it out. I have edited my response.

[chaseaucoin]

"Just to mention, it appears that the code is generated from service model JSON file."

So is the fix to update to the generator logic to handle nullable types correctly?

[ashishdhingra]

@chaseaucoin I will discuss with Dev team to get their inputs and will post any updates here.

[chaseaucoin]

This is currently a blocker for a handful of our products, if they don't have bandwidth I'm happy to put in a PR, let me know.

[chaseaucoin]

Looks like the issue is actually even worse than I originally thought and is likely affecting the Java SDK as well. The definition files that the generator are using have no notion of separation between nullable and non nullable integers or booleans. This means there are probably transient failures throughout the entire SDK for .NET and Java (at least)

https://github.com/aws/amazon-ecs-agent/blob/master/agent/acs/model/api/api-2.json

[debora-ito]

@chaseaucoin I tried to reproduce the issue using the Java SDK but I was not able to. I called RegisterTaskDefinition providing only the required fields (see code snippet below) and when I called DescribeTaskDefinition on this task I don't get any fields replaced by 0 or false.

ContainerDefinition definition = ContainerDefinition.builder()
     .name("my-container")
     .image("ubuntu")
     .build();

RegisterTaskDefinitionRequest request = RegisterTaskDefinitionRequest.builder()
     .requiresCompatibilities(Compatibility.FARGATE)
     .cpu("256")
     .memory("2048")
     .family("family")
     .networkMode("awsvpc")
     .containerDefinitions(definition)
     .build();

RegisterTaskDefinitionResponse response = client.registerTaskDefinition(request);

Can you provide a reproducible code sample of the issue you're seeing in the Java SDK?

[chaseaucoin]

@debora-ito yeah looks good. Looks like the model for java doesn't get generated the same way as .NET the nullable types are getting set as strings rather than the structure type specified in the root JSON model. Also looks like the Java SDK uses a builder pattern where the .NET library does not.

https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-ecs/src/main/java/com/amazonaws/services/ecs/model/Container.java

[normj]

@chaseaucoin how primitives are modeled in the AWS .NET SDK is a problem for use cases like this. Whenever we do a 4.0 version of the SDK I plan on pushing for us to use nullable (i.e. int?) for primitives. Unfortunately we can't do anything about it within the current major version of the SDK as it would be a massive breaking change.

A bit of history why the SDK is the way it is. As you have seen the SDK is generated from service models and the service models don't give a completely accurate information whether a field is required as well as services teams might start having a field be required but over time loosen the restriction as the service evolves. So we would have to make all primitives in the SDK by nullable. At least when we made that decision to not use nullable on the public interface, which was many years ago, we saw it was very uncommon to have nullable primitives on public interfaces and we went for simplicity. In hindsight I regret the decision for cases like you found. Java doesn't have this issue because using the boxed version of primitives has always been a common practice.

When the community asks we can turn on some helper methods that get outputted in the generator to let you know if a field actually has a value. That is the IsXSet methods you sometime see in the SDK. You would still have to call that IsXSet method first to see if you should copy a value from the describe response to the register request. Would that help you?

[chaseaucoin]

@normj

Thank you for the background.

" That is the IsXSet methods you sometime see in the SDK. You would still have to call that IsXSet method first to see if you should copy a value from the describe response to the register request. Would that help you?"

Unfortunately for this use case I don't believe so, if I'm understanding the implementation correctly, since I still wouldn't be able to register fargate tasks. It sounds like (please correct me if I am misunderstanding) IsXSet would help in reads, but not for serialization of the concreate object back to JSON which is ultimately what is giving me problem as I can't send valid values to the HTTP API.

[normj]

Yeah the IsXSet methods are not going to help you. You are going to have to customize the JSON serialization unfortunately.

If you are using Newtonsoft to convert to JSON you could create a custom resolver and for each of the problematic properties set a converter that checks to see if the value is the default for the type and if so return back null. We do something similar for our .NET Lambda Newtonsoft JSON serializers to handle base 64 encoded strings as .NET Streams. That might help you https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.Serialization.Json/AwsResolver.cs

Sorry again that this is a pain.

[github-actions]

We have noticed this issue has not recieved attention in a year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

[ashishdhingra]

Reopen until implementation

[github-actions]

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

[ashishdhingra]

No close

[normj]

We have a v4 in development that we hope to start releasing preview versions "soon". In v4 we are changing all properties using value types to use the nullable version and collections will default to null. This will remove the ambiguity between default values and what the service sends.

I'm closing this because this will has been addressed in the v4-development branch. Here is an example showing the container definition's cpu setting is an int?. https://github.com/aws/aws-sdk-net/blob/v4-development/sdk/src/Services/ECS/Generated/Model/ContainerDefinition.cs#L164

[github-actions]

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.