Skip to content

Commit

Permalink
chore(eks): extend support for clusterLogging disable
Browse files Browse the repository at this point in the history 
Amazon EKS Loggings was first introduced in #18112 and added Fargate
logging support in #20707. However, disabled loggings was not take into
consideration at the very beginning of the design.

With this PR, enabled clusterLogging support for enablement/disablement.
Also, added test cases for no logging configured, partial logging
configured, full logging configured.

Fixes: #19898
  • Loading branch information
@guessi
guessi committed Jul 17, 2022
1 parent 79cbe95 commit da58aa1
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 6 deletions.
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ function parseProps(props: any): aws.EKS.CreateClusterRequest {
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
View file
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 } ] };
readonly logging?: { [key: string]: [ { [key: string]: any }, { [key: string]: any } ] };
}

/**
Expand Down
18 changes: 17 additions & 1 deletion packages/@aws-cdk/aws-eks/lib/cluster.ts
View file
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 } ] };
private readonly logging?: { [key: string]: [ { [key: string]: any }, { [key: string]: any } ] };

/**
* A dummy CloudFormation resource that is used as a wait barrier which
Expand Down Expand Up @@ -1347,12 +1347,28 @@ 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: 16 additions & 0 deletions packages/@aws-cdk/aws-eks/test/cluster-resource-provider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,10 @@ describe('cluster resource provider', () => {
types: ['api'],
enabled: true,
},
{
types: ['audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: false,
},
],
},
}, {
Expand All @@ -581,6 +585,10 @@ describe('cluster resource provider', () => {
types: ['api'],
enabled: true,
},
{
types: ['audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: false,
},
],
},
});
Expand Down Expand Up @@ -622,6 +630,10 @@ describe('cluster resource provider', () => {
types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: true,
},
{
types: [],
enabled: false,
},
],
},
resourcesVpcConfig: {
Expand All @@ -644,6 +656,10 @@ describe('cluster resource provider', () => {
types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
enabled: true,
},
{
types: [],
enabled: false,
},
],
},
resourcesVpcConfig: {
Expand Down
94 changes: 94 additions & 0 deletions packages/@aws-cdk/aws-eks/test/cluster.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3156,4 +3156,98 @@ 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: [],
},
],
},
},
});
});
});
9 changes: 8 additions & 1 deletion ...s/@aws-cdk/aws-eks/test/eks-cluster.integ.snapshot/aws-cdk-eks-cluster-test.template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,13 @@
"authenticator",
"scheduler"
]
},
{
"enabled": false,
"types": [
"audit",
"controllerManager"
]
}
]
}
Expand Down Expand Up @@ -4032,4 +4039,4 @@
"Default": "/aws/service/bottlerocket/aws-k8s-1.21/x86_64/latest/image_id"
}
}
}
}
9 changes: 8 additions & 1 deletion ...s/@aws-cdk/aws-eks/test/eks-helm-asset.integ.snapshot/aws-cdk-eks-helm-test.template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,13 @@
"authenticator",
"scheduler"
]
},
{
"enabled": false,
"types": [
"audit",
"controllerManager"
]
}
]
}
Expand Down Expand Up @@ -1316,4 +1323,4 @@
"Description": "Artifact hash for asset \"3d78a5cdc39276c4ee8503417d4363951a0693b01cfd99ec9786feed456d012f\""
}
}
}
}
74 changes: 72 additions & 2 deletions packages/@aws-cdk/aws-eks/test/fargate.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,62 @@ describe('fargate', () => {

});

test('supports cluster logging with FargateCluster', () => {
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', () => {
// GIVEN
const stack = new Stack();

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

0 comments on commit da58aa1

Please sign in to comment.