feat(eks): control plane logs #8497

eduardomourar · 2020-06-11T13:13:26Z

This creates an option to enable the control plane logs during creation of EKS cluster. For more information, see documentation here.

Due to a limitation in the CloudFormation custom resources, we will allow booleans values to be encoded into special strings (true into 'TRUE:BOOLEAN' and false into 'FALSE:BOOLEAN').

closes #4159

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation · 2020-06-11T13:18:31Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e38402e
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-11T14:20:42Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 71f59a1
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb

If we want to implement this at a low level, I think that core.CustomResource should inject a special property to the custom resource which indicates that values are encoded and then automatically decode these automatically in core.CustomResourceProvider and custom-resources.Provider.

If this is too much, I am okay with just doing this at the EKS level for now.

packages/@aws-cdk/core/lib/custom-resource.ts

packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts

eladb · 2020-06-17T07:21:35Z

Ping?

eduardomourar · 2020-06-17T13:15:42Z

pong. sorry i will continue working on this

Pull request has been modified.

aws-cdk-automation · 2020-06-17T13:27:21Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 494330f
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-17T13:39:22Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e82e8a6
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-17T13:49:00Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 8b582d3
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-18T13:25:57Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e8bf3aa
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eduardomourar · 2020-06-18T14:08:11Z

If we want to implement this at a low level, I think that core.CustomResource should inject a special property to the custom resource which indicates that values are encoded and then automatically decode these automatically in core.CustomResourceProvider and custom-resources.Provider.

If this is too much, I am okay with just doing this at the EKS level for now.

I believe I covered all the code from the runtimes that you mentioned. Please, have a look.

aws-cdk-automation · 2020-06-19T23:30:15Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: f777ef4
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-20T09:28:32Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 0ec6571
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-20T10:14:17Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: e37d673
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-20T19:53:37Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: b15bdea
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Pull request has been modified.

aws-cdk-automation · 2020-06-23T12:40:17Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: a609ab9
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-23T13:08:28Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 5c70951
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb · 2020-06-23T14:54:21Z

packages/@aws-cdk/aws-eks/lib/cluster.ts

+   * Enable or disable exporting the Kubernetes control plane logs for your cluster to CloudWatch Logs.
+   *
+   * @default false Cluster control plane logs are not exported to CloudWatch Logs.
+   */


What happens if I enabled logs and then set to undefined (remove it). Would it set all the logging to false?

it will send an empty value for logging in the call to updateClusterConfig(), and it would result in setting the default value from AWS

Have you tested this? My intuition is that if I send an updateClusterConfig request with an empty value for logging it will not perform any changes to my logging configuration, which is not what users expect if they enable logs and then remove this property in their CDK app.

I suspect that we may either need to always send the two enabled/disable lists or add heuristics to the UPDATE handler to calculate the "diff" from the previous value and produce an appropriate update.

i will try this out and let you know, but this diff is being done in the runtime code here:

aws-cdk/packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts

Line 293 in 53d647f

updateLogging: JSON.stringify(newProps.logging) !== JSON.stringify(oldProps.logging),

This only determines that there was a change, not which log types should now be enabled/disabled based on that change.

For example, say, "oldProps" included two enabled types (say ["typeA", "typeB"]) and now "newProps" include only a single type (say ["typeB"]). Then, the request we need to send to updateClusterConfig is only to disable (we can, if we want also include an "enable" for typeB, but technically it's not required because it is already enabled).

LMK if this makes sense.

i believe we will need to send the whole configuration explicitly, including the disabled one. i am getting the following error when sending the exact same configuration to updateClusterConfig:

Failed to update resource. Error: No changes needed for the logging config provided

aws-cdk-automation · 2020-06-23T15:06:53Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 86af207
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-23T15:30:55Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: d538205
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-23T15:52:34Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 876a012
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-23T21:17:19Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: da23fc4
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/aws-eks/README.md

eladb

I suspect this is a bit more involved... See my comment about updates. I also still don't like the boolean encoding stuff.

Let's try to simplify. Instead of passing the convoluted EKS logging structure through the Config property to the custom resource provider, let's pass a separate logging configuration:

{
  Config: <no change>,
  EnabledLogTypes: string[],
  DisabledLogTypes: string[]
}

Then, in the provider, always produce a logging configuration based on these two arrays (no booleans(!)) and pass then to createCluster or updateClusterConfig.

Please also add unit tests to any changes made to the custom resource provider handler.

If you feel this is too much (and sorry for the hassle), feel free to abandon this PR and we will pick this up shortly. It is an important feature, so we will prioritize in the coming weeks.

eladb · 2020-06-24T06:03:32Z

packages/@aws-cdk/aws-eks/lib/cluster.ts

+   * Enable or disable exporting the Kubernetes control plane logs for your cluster to CloudWatch Logs.
+   *
+   * @default false Cluster control plane logs are not exported to CloudWatch Logs.
+   */


Have you tested this? My intuition is that if I send an updateClusterConfig request with an empty value for logging it will not perform any changes to my logging configuration, which is not what users expect if they enable logs and then remove this property in their CDK app.

I suspect that we may either need to always send the two enabled/disable lists or add heuristics to the UPDATE handler to calculate the "diff" from the previous value and produce an appropriate update.

aws-cdk-automation · 2020-06-24T06:39:57Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 4d65eb3
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-24T06:41:08Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 53d647f
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2020-06-24T06:41:24Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: abb5780
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eduardomourar · 2020-06-24T12:14:11Z

I do think this is a lot more complex than I expected, so I would rather abandon this PR (or the CDK can just pick from where I left off). Just a few things to consider:

The updateClusterConfig can have its own custom resource so that we have the control plane logging and the access endpoint ([aws-eks] Support choosing public, private or CIDR whitelist endpoints #5220) configuration decoupled from cluster creation process.
The boolean encoding/decoding is a recurring issue, and I see people are avoiding non strings in the custom resource providers being implemented because of that. So it should be somehow addressed from the CDK core and I am happy to contribute to it if needed be. For instance, having a JSON schema that can be used both in the custom resource construct and in the runtime to cast into proper types (which is similar to what CloudFormation resource provider schema attempts to do).

eladb · 2020-06-30T06:18:21Z

@eduardomourar the helm boolean flags for helm charts are being fixed by @pahud as part of #8787

Do we have other flags?

eduardomourar · 2020-06-30T10:01:08Z

I don’t believe we currently have any other flag, unless we introduce more parameters for the helm command.

adriantaut · 2021-09-24T14:13:21Z

@eladb I have a slightly different approach that I've just implemented in our company. Would like to get your pre-approval on the approach before preparing a PR, if this is possible.

import { AwsCustomResource, AwsCustomResourcePolicy } from "@aws-cdk/custom-resources";

const SUPPORTED_CONTROL_PLANE_LOGGING = ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'] as const;
type SUPPORTED_CONTROL_PLANE_LOGGING = typeof SUPPORTED_CONTROL_PLANE_LOGGING[number];

export interface EksControlPlaneLoggingProps {
  /**
   * Control Plane Logging according to https://docs.aws.amazon.com/eks/latest/userguide/control-plane-logs.html
   * If the `types` parameter is not explicitly provided, all the available Control Plane logging types will be enabled.
   *
   * @default false
   */
  readonly enabled: boolean;
  /**
   * Allows the enablement of only specific Control Plane Logging types
   */
  readonly types?: SUPPORTED_CONTROL_PLANE_LOGGING[];
}


export interface EksClusterProps {
.............................
  /**
   * EKS Control Plane Logging Configuration
   */
  readonly controlPlaneLogging?: EksControlPlaneLoggingProps,
.............................
}


/**
 * Base PPB Class for creating EKS CLusters
 */
export class EksCluster extends cdk.Construct {
  constructor(scope: cdk.Construct, id: string, props: EksClusterProps) {
    super(scope, id);

    .............................

    // instantiate the EKS Cluster
    this.cluster = this.createCluster(props, this.vpc);

    // add the NodeGroup according to user specifications
    this.nodeGroup = this.addNodeGroup(props.nodeGroup);

    // enable EKS Control Plane Logging if defined by user
    this.enableControlPlaneLogging(props.controlPlaneLogging);
    .............................
  }

  private enableControlPlaneLogging(props?: EksControlPlaneLoggingProps) {    
    if (props?.enabled) {
      const enabled_types = props.types ?? SUPPORTED_CONTROL_PLANE_LOGGING;
      const disabled_types = SUPPORTED_CONTROL_PLANE_LOGGING.filter(item => enabled_types.indexOf(item) < 0);

      new AwsCustomResource(this, "ClusterLogsEnabler", {
        policy: AwsCustomResourcePolicy.fromSdkCalls({
          resources: [`${this.cluster.clusterArn}/update-config`],
        }),
        onUpdate: {
          physicalResourceId: { id: `${this.cluster.clusterArn}/LogsEnabler` },
          service: "EKS",
          action: "updateClusterConfig",
          region: this.region,
          parameters: {
            name: this.cluster.clusterName,
            logging: {
              clusterLogging: [
                {
                  enabled: true,
                  types: enabled_types,
                },
                {
                  enabled: false,
                  types: disabled_types,
                }
              ],
            },
          },
        },
        onDelete: {
          physicalResourceId: { id: `${this.cluster.clusterArn}/LogsEnabler` },
          service: "EKS",
          action: "updateClusterConfig",
          region: this.region,
          parameters: {
            name: this.cluster.clusterName,
            logging: {
              clusterLogging: [
                {
                  enabled: false,
                  types: SUPPORTED_CONTROL_PLANE_LOGGING,
                },
              ],
            },
          },
        },
      });
    }
  }

It was tested in the following scenarios:

nothing is provided as controlPlaneLogging -> nothing touched, all the Loggings remain disabled
Enable all the loggings

      controlPlaneLogging: {
        enabled: true,
      },

Let user choose logging types to enable -> only user specified loggins enabled

      controlPlaneLogging: {
        enabled: true,
        types: ['api', 'audit'],
      },

Enable all or part of the loggings, then disable/completely remove the controlPlaneLogging reference -> disables all the loggings and put backs the default Disabled state

      controlPlaneLogging: {
        enabled: true,
      },

followed by completely removing the controlPlaneLogging reference or:

      controlPlaneLogging: {
        enabled: false,
      },

Looking forward for your response, cheers!

support encoding for boolean values

e38402e

Merge branch 'master' into feature/enable-control-plane-logs

71f59a1

SomayaB assigned eladb Jun 11, 2020

eladb previously requested changes Jun 15, 2020

View reviewed changes

packages/@aws-cdk/core/lib/custom-resource.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/common.ts Show resolved Hide resolved

improve description custom resource properties

494330f

eduardomourar added 2 commits June 17, 2020 15:34

remove decode boolean from eks

e82e8a6

fix linting

8b582d3

add decode booleans to runtime code

e8bf3aa

eduardomourar added 3 commits June 20, 2020 01:22

add unit tests in core lib

83f0cb4

encode values for helm charts

72226b0

add support for control plane logging

f777ef4

update hash on core test

0ec6571

update integ for iam

e37d673

eduardomourar added 2 commits June 20, 2020 14:17

Merge 'master' into feature branch

a250704

increase unit test coverage

b15bdea

update readme

8c79e70

add helm wait comment

a609ab9

revert back all changes to runtime code

5c70951

eduardomourar and others added 2 commits June 23, 2020 16:22

update test files

86af207

Update integ.core-custom-resources.expected.json

d538205

eladb reviewed Jun 23, 2020

View reviewed changes

Merge branch 'master' into feature/enable-control-plane-logs

876a012

Merge branch 'master' into feature/enable-control-plane-logs

da23fc4

Update integ.core-custom-resources.expected.json

4d65eb3

eladb reviewed Jun 24, 2020

View reviewed changes

packages/@aws-cdk/aws-eks/README.md Outdated Show resolved Hide resolved

Elad Ben-Israel added 2 commits June 24, 2020 08:56

Update packages/@aws-cdk/aws-eks/README.md

abb5780

Reorg file

53d647f

eladb suggested changes Jun 24, 2020

View reviewed changes

eladb changed the title ~~feat(eks): enable control plane logs~~ feat(eks): control plane logs Jun 24, 2020

eladb closed this Jul 2, 2020

eladb mentioned this pull request Jul 27, 2020

feat(eks): endpoint access customization #9095

Merged

iliapolo mentioned this pull request Aug 16, 2020

[aws-eks] Enable Control Plane logs in EKS cluster #4159

Closed

feat(eks): control plane logs #8497

feat(eks): control plane logs #8497

Conversation

eduardomourar commented Jun 11, 2020

aws-cdk-automation commented Jun 11, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 11, 2020

AWS CodeBuild CI Report

eladb left a comment

Choose a reason for hiding this comment

eladb commented Jun 17, 2020

eduardomourar commented Jun 17, 2020

aws-cdk-automation commented Jun 17, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 17, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 17, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 18, 2020

AWS CodeBuild CI Report

eduardomourar commented Jun 18, 2020 • edited Loading

aws-cdk-automation commented Jun 19, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 20, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 20, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 20, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

eladb Jun 23, 2020

Choose a reason for hiding this comment

eduardomourar Jun 23, 2020

Choose a reason for hiding this comment

eladb Jun 24, 2020

Choose a reason for hiding this comment

eduardomourar Jun 24, 2020

Choose a reason for hiding this comment

eladb Jun 24, 2020

Choose a reason for hiding this comment

eduardomourar Jun 24, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 23, 2020

AWS CodeBuild CI Report

eladb left a comment

Choose a reason for hiding this comment

eladb Jun 24, 2020

Choose a reason for hiding this comment

aws-cdk-automation commented Jun 24, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 24, 2020

AWS CodeBuild CI Report

aws-cdk-automation commented Jun 24, 2020

AWS CodeBuild CI Report

eduardomourar commented Jun 24, 2020 • edited Loading

eladb commented Jun 30, 2020

eduardomourar commented Jun 30, 2020

adriantaut commented Sep 24, 2021

eduardomourar commented Jun 18, 2020 •

edited

Loading

eduardomourar commented Jun 24, 2020 •

edited

Loading