Skip to content

Commit

Permalink
fix(eks): fail to update cluster by disabling logging props (#24688)
Browse files Browse the repository at this point in the history
* fix

* update path

* delete
  • Loading branch information
pahud authored Mar 21, 2023
1 parent 413b643 commit 767cf93
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/l
// eslint-disable-next-line import/no-extraneous-dependencies
import * as aws from 'aws-sdk';
import { EksClient, ResourceEvent, ResourceHandler } from './common';
import { compareLoggingProps } from './compareLogging';


const MAX_CLUSTER_NAME_LEN = 100;

Expand All @@ -25,6 +27,9 @@ export class ClusterResourceHandler extends ResourceHandler {

this.newProps = parseProps(this.event.ResourceProperties);
this.oldProps = event.RequestType === 'Update' ? parseProps(event.OldResourceProperties) : {};
// compare newProps and oldProps and update the newProps by appending disabled LogSetup if any
const compared: Partial<aws.EKS.CreateClusterRequest> = compareLoggingProps(this.oldProps, this.newProps);
this.newProps.logging = compared.logging;
}

// ------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* This function compares the logging configuration from oldProps and newProps and returns
* the result that contains LogSetup with enabled:false if any.
*
* @param oldProps old properties
* @param newProps new properties
* @returns result with LogSet with enabled:false if any
*/

export function compareLoggingProps(oldProps: Partial<AWS.EKS.CreateClusterRequest>,
newProps: Partial<AWS.EKS.CreateClusterRequest>): Partial<AWS.EKS.CreateClusterRequest> {
const result: Partial<AWS.EKS.CreateClusterRequest> = { logging: {} };
let enabledTypes: AWS.EKS.LogType[] = [];
let disabledTypes: AWS.EKS.LogType[] = [];

if (newProps.logging?.clusterLogging === undefined && oldProps.logging?.clusterLogging === undefined) {
return newProps;
}
// if newProps containes LogSetup
if (newProps.logging && newProps.logging.clusterLogging && newProps.logging.clusterLogging.length > 0) {
enabledTypes = newProps.logging.clusterLogging[0].types!;
// if oldProps contains LogSetup with enabled:true
if (oldProps.logging && oldProps.logging.clusterLogging && oldProps.logging.clusterLogging.length > 0) {
// LogType in oldProp but not in newProp should be considered disabled(enabled:false)
disabledTypes = oldProps.logging!.clusterLogging![0].types!.filter(t => !newProps.logging!.clusterLogging![0].types!.includes(t));
}
} else {
// all enabled:true in oldProps will be enabled:false
disabledTypes = oldProps.logging!.clusterLogging![0].types!;
}

if (enabledTypes.length > 0 || disabledTypes.length > 0) {
result.logging = { clusterLogging: [] };
}

// append the enabled:false LogSetup to the result
if (enabledTypes.length > 0) {
result.logging!.clusterLogging!.push({ types: enabledTypes, enabled: true });
}
// append the enabled:false LogSetup to the result
if (disabledTypes.length > 0) {
result.logging!.clusterLogging!.push({ types: disabledTypes, enabled: false });
}
return result;
}
89 changes: 89 additions & 0 deletions packages/@aws-cdk/aws-eks/test/compareLog.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import * as aws from 'aws-sdk';
import * as eks from '../lib';
import { compareLoggingProps } from '../lib/cluster-resource-handler/compareLogging';

describe('compareLoggingProps', () => {

type Props = Partial<aws.EKS.CreateClusterRequest>;
const oldEnabledTypes: aws.EKS.LogTypes = [eks.ClusterLoggingTypes.API, eks.ClusterLoggingTypes.AUDIT];

test('when newProps.logging.clusterLogging is undefined, should disable all types with enabled:true in oldProps', () => {
const oldProps: Props = {
logging: {
clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
},
};

const newProps: Props = {
logging: {},
};

const result = compareLoggingProps(oldProps, newProps);

expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
});

test('when newProps.logging is undefined, should disable all types with enabled:true in oldProps', () => {
const oldProps: Props = {
logging: {
clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
},
};

const newProps: Props = {};

const result = compareLoggingProps(oldProps, newProps);

expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
});

test('should disable types with enabled:true but not defined as enabled:true in newProps', () => {
const oldProps: Props = {
logging: {
clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
},
};

const newProps: Props = {
logging: {
clusterLogging: [{ types: [eks.ClusterLoggingTypes.AUDIT], enabled: true }],
},
};

const result = compareLoggingProps(oldProps, newProps);

expect(result.logging?.clusterLogging).toEqual([{ types: [eks.ClusterLoggingTypes.AUDIT], enabled: true },
{ types: [eks.ClusterLoggingTypes.API], enabled: false }]);
});

test('when oldProps.logging.clusterLogging is undefined and newProps.logging.clusterLogging is undefined, result should be newProps', () => {
const oldProps: Props = {
logging: {},
};

const newProps: Props = {
logging: {},
};

const result = compareLoggingProps(oldProps, newProps);

expect(result).toEqual(newProps);
});

test('multiple enabled:true types in oldProps with clusterLogging undefined in newProps should all be disabled', () => {
const oldProps: Props = {
logging: {
clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
},
};

const newProps: Props = {
logging: {},
};

const result = compareLoggingProps(oldProps, newProps);

expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
});

});

0 comments on commit 767cf93

Please sign in to comment.