Skip to content

Commit

Permalink
revert "fix(eks): cannot disable cluster logging once it has been ena…
Browse files Browse the repository at this point in the history
…bled" (#21545)

Reverts #21185 and #21463
Closes #21515
Re-opens #19898. Fix for this in progress.

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
TheRealAmazonKendra committed Aug 10, 2022
1 parent 7e2fe56 commit 5515ce4
Show file tree
Hide file tree
Showing 8 changed files with 7 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -286,14 +286,10 @@ function parseProps(props: any): aws.EKS.CreateClusterRequest {
parsed.resourcesVpcConfig.endpointPublicAccess = parsed.resourcesVpcConfig.endpointPublicAccess === 'true';
}

if (typeof (parsed.logging?.clusterLogging[0]?.enabled) === 'string') {
if (typeof (parsed.logging?.clusterLogging[0].enabled) === 'string') {
parsed.logging.clusterLogging[0].enabled = parsed.logging.clusterLogging[0].enabled === 'true';
}

if (typeof (parsed.logging?.clusterLogging[1]?.enabled) === 'string') {
parsed.logging.clusterLogging[1].enabled = parsed.logging.clusterLogging[1].enabled === 'false';
}

return parsed;

}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-eks/lib/cluster-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface ClusterResourceProps {
readonly onEventLayer?: lambda.ILayerVersion;
readonly clusterHandlerSecurityGroup?: ec2.ISecurityGroup;
readonly tags?: { [key: string]: string };
readonly logging?: { [key: string]: [ { [key: string]: any }, { [key: string]: any } ] };
readonly logging?: { [key: string]: [ { [key: string]: any } ] };
}

/**
Expand Down
18 changes: 1 addition & 17 deletions packages/@aws-cdk/aws-eks/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1285,7 +1285,7 @@ export class Cluster extends ClusterBase {

private readonly version: KubernetesVersion;

private readonly logging?: { [key: string]: [ { [key: string]: any }, { [key: string]: any } ] };
private readonly logging?: { [key: string]: [ { [key: string]: any } ] };

/**
* A dummy CloudFormation resource that is used as a wait barrier which
Expand Down Expand Up @@ -1347,28 +1347,12 @@ export class Cluster extends ClusterBase {
// Get subnetIds for all selected subnets
const subnetIds = Array.from(new Set(flatten(selectedSubnetIdsPerGroup)));

// The value of clusterLoggingTypeDisabled should be invert of props.clusterLogging.
let clusterLoggingTypeDisabled: ClusterLoggingTypes[] = [];

// Find out type(s) to disable.
Object.values(ClusterLoggingTypes).forEach(function (key) {
let clusterLoggingTypeEnabled = Object.values(props.clusterLogging ? Object.values(props.clusterLogging) : []);
if (!Object.values(clusterLoggingTypeEnabled).includes(key)) {
clusterLoggingTypeDisabled.push(key);
};
});

// Leave it untouched as undefined if (props.clusterLogging === undefined).
this.logging = props.clusterLogging ? {
clusterLogging: [
{
enabled: true,
types: Object.values(props.clusterLogging),
},
{
enabled: false,
types: Object.values(clusterLoggingTypeDisabled),
},
],
} : undefined;

Expand Down
16 changes: 0 additions & 16 deletions packages/@aws-cdk/aws-eks/test/cluster-resource-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,10 +566,6 @@ describe('cluster resource provider', () => {
types: ['api'],
enabled: true,
},
{
types: ['audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: false,
},
],
},
}, {
Expand All @@ -585,10 +581,6 @@ describe('cluster resource provider', () => {
types: ['api'],
enabled: true,
},
{
types: ['audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: false,
},
],
},
});
Expand Down Expand Up @@ -630,10 +622,6 @@ describe('cluster resource provider', () => {
types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: true,
},
{
types: [],
enabled: false,
},
],
},
resourcesVpcConfig: {
Expand All @@ -656,10 +644,6 @@ describe('cluster resource provider', () => {
types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: true,
},
{
types: [],
enabled: false,
},
],
},
resourcesVpcConfig: {
Expand Down
94 changes: 0 additions & 94 deletions packages/@aws-cdk/aws-eks/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3156,98 +3156,4 @@ describe('cluster', () => {
},
});
});

test('create a cluster without logging configure', () => {
// GIVEN
const { stack } = testFixture();

// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
});

// THEN
Template.fromStack(stack).resourceCountIs('Custom::AWSCDK-EKS-Cluster::Config::logging', 0);
});

test('create a cluster with partial logging configure', () => {
// GIVEN
const { stack } = testFixture();

// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
clusterLogging: [
eks.ClusterLoggingTypes.API,
eks.ClusterLoggingTypes.AUTHENTICATOR,
eks.ClusterLoggingTypes.SCHEDULER,
],
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
logging: {
clusterLogging: [
{
enabled: true,
types: [
'api',
'authenticator',
'scheduler',
],
},
{
enabled: false,
types: [
'audit',
'controllerManager',
],
},
],
},
},
});
});

test('create a cluster with all logging configure', () => {
// GIVEN
const { stack } = testFixture();

// WHEN
new eks.Cluster(stack, 'Cluster', {
version: CLUSTER_VERSION,
clusterLogging: [
eks.ClusterLoggingTypes.API,
eks.ClusterLoggingTypes.AUDIT,
eks.ClusterLoggingTypes.AUTHENTICATOR,
eks.ClusterLoggingTypes.CONTROLLER_MANAGER,
eks.ClusterLoggingTypes.SCHEDULER,
],
});

// THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
logging: {
clusterLogging: [
{
enabled: true,
types: [
'api',
'audit',
'authenticator',
'controllerManager',
'scheduler',
],
},
{
enabled: false,
types: [],
},
],
},
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -825,13 +825,6 @@
"authenticator",
"scheduler"
]
},
{
"enabled": false,
"types": [
"audit",
"controllerManager"
]
}
]
}
Expand Down Expand Up @@ -4039,4 +4032,4 @@
"Default": "/aws/service/bottlerocket/aws-k8s-1.21/x86_64/latest/image_id"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -668,13 +668,6 @@
"authenticator",
"scheduler"
]
},
{
"enabled": false,
"types": [
"audit",
"controllerManager"
]
}
]
}
Expand Down Expand Up @@ -1323,4 +1316,4 @@
"Description": "Artifact hash for asset \"3d78a5cdc39276c4ee8503417d4363951a0693b01cfd99ec9786feed456d012f\""
}
}
}
}
74 changes: 2 additions & 72 deletions packages/@aws-cdk/aws-eks/test/fargate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,62 +459,7 @@ describe('fargate', () => {

});

test('supports cluster logging without FargateCluster', () => {
// GIVEN
const stack = new Stack();

// WHEN

new eks.FargateCluster(stack, 'FargateCluster', {
version: CLUSTER_VERSION,
});

//THEN
Template.fromStack(stack).resourceCountIs('Custom::AWSCDK-EKS-Cluster::Config::logging', 0);
});

test('supports cluster partial logging enabled with FargateCluster', () => {
// GIVEN
const stack = new Stack();

// WHEN

new eks.FargateCluster(stack, 'FargateCluster', {
version: CLUSTER_VERSION,
clusterLogging: [
eks.ClusterLoggingTypes.API,
eks.ClusterLoggingTypes.AUTHENTICATOR,
eks.ClusterLoggingTypes.SCHEDULER,
],
});

//THEN
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
Config: {
logging: {
clusterLogging: [
{
enabled: true,
types: [
'api',
'authenticator',
'scheduler',
],
},
{
enabled: false,
types: [
'audit',
'controllerManager',
],
},
],
},
},
});
});

test('supports cluster all logging enabled with FargateCluster', () => {
test('supports cluster logging with FargateCluster', () => {
// GIVEN
const stack = new Stack();

Expand All @@ -524,9 +469,7 @@ describe('fargate', () => {
version: CLUSTER_VERSION,
clusterLogging: [
eks.ClusterLoggingTypes.API,
eks.ClusterLoggingTypes.AUDIT,
eks.ClusterLoggingTypes.AUTHENTICATOR,
eks.ClusterLoggingTypes.CONTROLLER_MANAGER,
eks.ClusterLoggingTypes.SCHEDULER,
],
});
Expand All @@ -536,20 +479,7 @@ describe('fargate', () => {
Config: {
logging: {
clusterLogging: [
{
enabled: true,
types: [
'api',
'audit',
'authenticator',
'controllerManager',
'scheduler',
],
},
{
enabled: false,
types: [],
},
{ enabled: true, types: ['api', 'authenticator', 'scheduler'] },
],
},
},
Expand Down

0 comments on commit 5515ce4

Please sign in to comment.