[aws-eks] ServiceAccount property references do not result in dependencies #9910

otterley · 2020-08-22T17:49:39Z

CDK does not currently establish any sort of dependency--implicit or otherwise--between a ServiceAccount construct and any EKS resources that refer to its properties, such as HelmCharts and KubernetesManifests. This can result in the service account being destroyed before a dependent CRD controller is destroyed. This can cause ordering failures on stack destruction.

Consider the following sequence of events:

ServiceAccount is destroyed
Controller (which uses the service account) is still running but all operations will fail
CRD (which depends on controller's proper operation) cannot be successfully deleted

At root, the issue seems to be that ServiceAccounts are not associated with first-class Custom Resources. The name and namespace parameters are not formal CloudFormation parameters, which can be used as references by other CF Resources. Instead, an ordinary KubernetesResource is created, and the name and namespace parameters are interned into the K8S resource JSON.

Nor does CDK create any explicit dependencies between a service account and its consumer by way of a DependsOn key in the rendered template.

❗ In fact, further testing shows that it is not even possible to explicitly declare a dependency between a ServiceAccount and a dependent resource. No error results, but no dependency is created, either.

Reproduction Steps

      const appMeshControllerServiceAccount = cluster.addServiceAccount(
        "AppMesh",
        {
          name: "appmesh-controller",
          namespace: "appmesh-system",
        }
      );

      const appMeshController = cluster.addChart("appmesh-controller", {
        chart: "appmesh-controller",
        repository: "https://aws.github.io/eks-charts",
        namespace: "appmesh-system",
        createNamespace: false,
        release: "appmesh-controller",
        values: {
          region: this.region,
          serviceAccount: {
            create: false,
            // Expectation is that the following creates an implicit dependency
            name: appMeshControllerServiceAccount.serviceAccountName,
          },
        },
        wait: true,
      });

Rendered template fragment:

"ClusterAmanifestAppMeshServiceAccountResourceAABB6973": {
      "Type": "Custom::AWSCDK-EKS-KubernetesResource",
      "Properties": {
        "ServiceToken": {
          "Fn::GetAtt": [
            "awscdkawseksKubectlProviderNestedStackawscdkawseksKubectlProviderNestedStackResourceA7AEBA6B",
            "Outputs.LocalZoneAppMeshRoutingStackawscdkawseksKubectlProviderframeworkonEvent9E9D6AA9Arn"
          ]
        },
        "Manifest": {
          "Fn::Join": [
            "",
            [
              "[{\"apiVersion\":\"v1\",\"kind\":\"ServiceAccount\",\"metadata\":{\"name\":\"appmesh-controller\",\"namespace\":\"appmesh-system\",\"labels\":{\"app.kubernetes.io/name\":\"appmesh-controller\"},\"annotations\":{\"eks.amazonaws.com/role-arn\":\"",
              {
                "Fn::GetAtt": [
                  "ClusterAAppMeshRole998D0403",
                  "Arn"
                ]
              },
              "\"}}}]"
            ]
          ]
        },
        "ClusterName": {
          "Ref": "ClusterA66FB302D"
        },
        "RoleArn": {
          "Fn::GetAtt": [
            "ClusterACreationRoleDCD3F752",
            "Arn"
          ]
        }
      },
      "DependsOn": [
        "ClusterAKubectlReadyBarrier407D31C8"
      ],
      "UpdateReplacePolicy": "Delete",
      "DeletionPolicy": "Delete",
      "Metadata": {
        "aws:cdk:path": "LocalZoneAppMeshRoutingStack/ClusterA/manifest-AppMeshServiceAccountResource/Resource/Default"
      }
    },
    "ClusterAchartappmeshcontrollerBC62F8B6": {
      "Type": "Custom::AWSCDK-EKS-HelmChart",
      "Properties": {
        "ServiceToken": {
          "Fn::GetAtt": [
            "awscdkawseksKubectlProviderNestedStackawscdkawseksKubectlProviderNestedStackResourceA7AEBA6B",
            "Outputs.LocalZoneAppMeshRoutingStackawscdkawseksKubectlProviderframeworkonEvent9E9D6AA9Arn"
          ]
        },
        "ClusterName": {
          "Ref": "ClusterA66FB302D"
        },
        "RoleArn": {
          "Fn::GetAtt": [
            "ClusterACreationRoleDCD3F752",
            "Arn"
          ]
        },
        "Release": "appmesh-controller",
        "Chart": "appmesh-controller",
        "Wait": true,
        "Values": "{\"region\":\"us-east-1\",\"serviceAccount\":{\"create\":false,\"name\":\"appmesh-controller\"}}",
        "Namespace": "appmesh-system",
        "Repository": "https://aws.github.io/eks-charts"
      },
      "DependsOn": [
        "ClusterAKubectlReadyBarrier407D31C8",
        "ClusterAmanifestAppMeshSystemNamespace65398EC6",
        "ClusterANodegroupNodeGroupRoleBC94C691",
        "ClusterANodegroup47FA0B14"
      ],
      "UpdateReplacePolicy": "Delete",
      "DeletionPolicy": "Delete",
      "Metadata": {
        "aws:cdk:path": "LocalZoneAppMeshRoutingStack/ClusterA/chart-appmesh-controller/Resource/Default"
      }
    },

What did you expect to happen?

I expected an implicit dependency to be created on the serviceAccount by the chart.

What actually happened?

No such dependency was created.

Environment

CLI Version : 1.59.0
Framework Version: 1.59.0
Node.js Version: 12.18.2
OS : MacOS Catalina
Language (Version): Typescript 3.7.5

Other

This is 🐛 Bug Report

The text was updated successfully, but these errors were encountered:

otterley · 2020-08-22T19:38:51Z

Added edit above:

❗ In fact, further testing shows that it is not even possible to explicitly declare a dependency between a ServiceAccount and a dependent resource. No error results, but no dependency is created, either.

iliapolo · 2020-08-23T09:02:56Z

@otterley Thats an interesting issue.

Implicit dependencies between resources are created when one resource uses an attribute of another resource, in the form of a token. In this case, while it looks like one, appMeshControllerServiceAccount.serviceAccountName is not actually an attribute, it's just a literal string. So what's really written here is:

const appMeshController = cluster.addChart("appmesh-controller", {
        chart: "appmesh-controller",
        repository: "https://aws.github.io/eks-charts",
        namespace: "appmesh-system",
        createNamespace: false,
        release: "appmesh-controller",
        values: {
          region: this.region,
          serviceAccount: {
            create: false,
            // Expectation is that the following creates an implicit dependency
            name: 'appmesh-controller',
          },
        },
        wait: true,
      });

And in fact there is no real reference between the controller and the service account.

Having said that, I do agree its confusing, and needed. One solution to this would be to actually have serviceAccount.serviceAccountName be a token that fetches the service account name by using the KubernetesObjectValue construct.

This would force the retrieval of .serviceAccountName to wait until the resource is actually created, but also feels like kind of a stretch since in many cases that won't be necessary.

I'm going to mark this as a feature request rather than a bug.

In the meantime, one can declare an explicit dependency:

appMeshController.node.addDependency(appMeshControllerServiceAccount)

iliapolo · 2020-08-23T16:20:27Z

In the meantime, one can declare an explicit dependency:

appMeshController.node.addDependency(appMeshControllerServiceAccount)

Correction

As mentioned by @otterley, even declaring this dependency directly will not work because of #8884. The service account manifest resource itself is not added as a child of the ServiceAccount construct so its not being considered in the dependency resolution.

Verified that this PR will fix that as well.

As a workaround for now, you can locate the inner resource and add it as a dependency as well:

const appMeshControllerServiceAccountManifestResource = appMeshControllerServiceAccount.node.findChild('manifest-AppMeshServiceAccountResource');
appMeshController.node.addDependency(appMeshControllerServiceAccount);
appMeshController.node.addDependency(appMeshControllerServiceAccountManifestResource);

otterley · 2020-08-23T19:16:35Z

One solution to this would be to actually have serviceAccount.serviceAccountName be a token that fetches the service account name by using the KubernetesObjectValue construct.

This would force the retrieval of .serviceAccountName to wait until the resource is actually created, but also feels like kind of a stretch since in many cases that won't be necessary.

I disagree that it's unnecessary. While it may be unnecessary for stack creation, it is quite often needed for stack destruction to work properly. Any K8S CRD controller that relies on a service account to work properly must not have its service account destroyed before the controller itself is. Otherwise it will be impossible to delete the CRDs that are managed by the controller.

kishoreallwynraj · 2020-11-15T11:14:42Z

@iliapolo I am also facing the same issue and i tried the workaround you mentioned in the issue. But i am getting error during synth process that "No child with id". I have added the details under issue #11475

github-actions · 2022-06-28T00:22:12Z

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

otterley added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 22, 2020

github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Aug 22, 2020

github-actions bot assigned iliapolo Aug 22, 2020

otterley changed the title ~~[aws-eks] Resources (charts, manifests, etc.) do not have proper dependency on service account~~ [aws-eks] ServiceAccount property references do not result in dependencies Aug 22, 2020

otterley mentioned this issue Aug 22, 2020

[aws-eks] All eks.ServiceAccount constructs Are Added to Cluster Stack #9325

Closed

iliapolo added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 and removed needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. labels Aug 23, 2020

iliapolo mentioned this issue Sep 15, 2020

fix(eks): creating a ServiceAccount in a different stack than the Cluster creates circular dependency between the two stacks #9701

Merged

kishoreallwynraj mentioned this issue Nov 14, 2020

[aws-eks] Values passed to AddHelmChart are not getting applied #11475

Closed

iliapolo removed their assignment Jun 27, 2021

github-actions bot closed this as completed Jul 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[aws-eks] ServiceAccount property references do not result in dependencies #9910

[aws-eks] ServiceAccount property references do not result in dependencies #9910

otterley commented Aug 22, 2020 •

edited

Loading

otterley commented Aug 22, 2020

iliapolo commented Aug 23, 2020

iliapolo commented Aug 23, 2020 •

edited

Loading

otterley commented Aug 23, 2020

kishoreallwynraj commented Nov 15, 2020

github-actions bot commented Jun 28, 2022

[aws-eks] ServiceAccount property references do not result in dependencies #9910

[aws-eks] ServiceAccount property references do not result in dependencies #9910

Comments

otterley commented Aug 22, 2020 • edited Loading

Reproduction Steps

What did you expect to happen?

What actually happened?

Environment

Other

otterley commented Aug 22, 2020

iliapolo commented Aug 23, 2020

iliapolo commented Aug 23, 2020 • edited Loading

otterley commented Aug 23, 2020

kishoreallwynraj commented Nov 15, 2020

github-actions bot commented Jun 28, 2022

otterley commented Aug 22, 2020 •

edited

Loading

iliapolo commented Aug 23, 2020 •

edited

Loading