Skip to content

Commit

Permalink
feat(aws-eks): adding takeOwnership option to HelmChart construct
Browse files Browse the repository at this point in the history
  • Loading branch information
hanseltime committed Jan 19, 2025
1 parent 53dc0d8 commit b7dc8eb
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class EksClusterStack extends Stack {
vpc: this.vpc,
mastersRole,
defaultCapacity: 2,
...getClusterVersionConfig(this),
...getClusterVersionConfig(this, eks.KubernetesVersion.V1_31),
tags: {
foo: 'bar',
},
Expand Down Expand Up @@ -108,6 +108,21 @@ class EksClusterStack extends Stack {
values: { aws: { region: this.region } },
});

// testing installation with --take-ownership flag set to true
// https://gallery.ecr.aws/aws-controllers-k8s/sns-chart
this.cluster.addHelmChart('test-take-ownership-installation', {
chart: 'sns-chart',
release: 'sns-chart-release',
repository: 'oci://public.ecr.aws/aws-controllers-k8s/sns-chart',
version: '1.1.2',
namespace: 'ask-system',
createNamespace: true,
skipCrds: true,
atomic: true,
takeOwnership: true,
values: { aws: { region: this.region } },
});

// https://github.com/orgs/grafana-operator/packages/container/package/helm-charts%2Fgrafana-operator
this.cluster.addHelmChart('test-non-ecr-oci-chart', {
chart: 'grafana-operator',
Expand Down
10 changes: 10 additions & 0 deletions packages/@aws-cdk/aws-eks-v2-alpha/lib/helm-chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ export interface HelmChartOptions {
* @default - CRDs are installed if not already present
*/
readonly skipCrds?: boolean;

/**
* if set, this will use the --take-ownership flag provided since helm >=v3.17.0. Helm will not throw an error if any manifest
* that it renders if already on the cluster and will instead overwrite and annotate it with ownership from the rendered manifest
* @default false
*/
readonly takeOwnership?: boolean;
}

/**
Expand Down Expand Up @@ -158,6 +165,8 @@ export class HelmChart extends Construct {
const skipCrds = props.skipCrds ?? false;
// default to set atomic as false
const atomic = props.atomic ?? false;
// default to not take ownership since helm only only added this as of 3.17.0
const takeOwnership = props.takeOwnership ?? false;

this.chartAsset?.grantRead(provider.handlerRole);

Expand All @@ -179,6 +188,7 @@ export class HelmChart extends Construct {
CreateNamespace: createNamespace || undefined,
SkipCrds: skipCrds || undefined,
Atomic: atomic || undefined, // props are stringified so we encode “false” as undefined
TakeOwnership: takeOwnership || undefined,
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def helm_handler(event, context):
repository = props.get('Repository', None)
values_text = props.get('Values', None)
skip_crds = props.get('SkipCrds', False)
take_ownership = props.get('TakeOwnership', False)

# "log in" to the cluster
subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',
Expand Down Expand Up @@ -91,7 +92,7 @@ def helm_handler(event, context):
chart_dir = get_chart_from_oci(tmpdir.name, repository, version)
chart = chart_dir

helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace, atomic=atomic)
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace, atomic=atomic, take_ownership=take_ownership)
elif request_type == "Delete":
try:
helm('uninstall', release, namespace=namespace, wait=wait, timeout=timeout)
Expand Down Expand Up @@ -158,7 +159,7 @@ def get_chart_from_oci(tmpdir, repository = None, version = None):
raise Exception(f'Operation failed after {maxAttempts} attempts: {output}')


def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None, create_namespace = None, skip_crds = False, atomic = False):
def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None, create_namespace = None, skip_crds = False, atomic = False, take_ownership = False):
import subprocess

cmnd = ['helm', verb, release]
Expand All @@ -183,7 +184,9 @@ def helm(verb, release, chart = None, repo = None, file = None, namespace = None
if not timeout is None:
cmnd.extend(['--timeout', timeout])
if atomic:
cmnd.append('--atomic')
cmnd.append('--atomic')
if take_ownership:
cmnd.append('--take-ownership')
cmnd.extend(['--kubeconfig', kubeconfig])

maxAttempts = 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def helm_handler(event, context):
repository = props.get('Repository', None)
values_text = props.get('Values', None)
skip_crds = props.get('SkipCrds', False)
take_ownership = props.get('TakeOwnership', False)

# "log in" to the cluster
subprocess.check_call([ 'aws', 'eks', 'update-kubeconfig',
Expand Down Expand Up @@ -91,7 +92,7 @@ def helm_handler(event, context):
chart_dir = get_chart_from_oci(tmpdir.name, repository, version)
chart = chart_dir

helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace, atomic=atomic)
helm('upgrade', release, chart, repository, values_file, namespace, version, wait, timeout, create_namespace, atomic=atomic, take_ownership=take_ownership)
elif request_type == "Delete":
try:
helm('uninstall', release, namespace=namespace, wait=wait, timeout=timeout)
Expand Down Expand Up @@ -158,7 +159,7 @@ def get_chart_from_oci(tmpdir, repository = None, version = None):
raise Exception(f'Operation failed after {maxAttempts} attempts: {output}')


def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None, create_namespace = None, skip_crds = False, atomic = False):
def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None, create_namespace = None, skip_crds = False, atomic = False, take_ownership = False):
import subprocess

cmnd = ['helm', verb, release]
Expand All @@ -183,7 +184,9 @@ def helm(verb, release, chart = None, repo = None, file = None, namespace = None
if not timeout is None:
cmnd.extend(['--timeout', timeout])
if atomic:
cmnd.append('--atomic')
cmnd.append('--atomic')
if take_ownership:
cmnd.append('--take-ownership')
cmnd.extend(['--kubeconfig', kubeconfig])

maxAttempts = 3
Expand Down
19 changes: 18 additions & 1 deletion packages/aws-cdk-lib/aws-eks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ In addition, the library also supports defining Kubernetes resource manifests wi
- [Permissions and Security](#permissions-and-security)
- [AWS IAM Mapping](#aws-iam-mapping)
- [Access Config](#access-config)
- [Access Entry](#access-mapping)
- [Access Entry](#access-entry)
- [Migrating from ConfigMap to Access Entry](#migrating-from-configmap-to-access-entry)
- [Cluster Security Group](#cluster-security-group)
- [Node SSH Access](#node-ssh-access)
- [Service Accounts](#service-accounts)
Expand Down Expand Up @@ -1622,6 +1623,22 @@ new eks.HelmChart(this, 'NginxIngress', {
});
```

Helm chart can also execute upgrades with the `--take-ownership` flag as long as the cluster's kubectl layer has `helm >=3.17.0`.
This option is useful if you are using a tool or development pattern where manifests might be added separately via kubectl during
something like a migration. Without this option set, your helm chart construct will fail to deploy by if it cannot detect the helm-applied annotations on existing manifests.

```ts
declare const cluster: eks.Cluster;
// option 1: use a construct
new eks.HelmChart(this, 'NginxIngress', {
cluster,
chart: 'nginx-ingress',
repository: 'https://helm.nginx.com/stable',
namespace: 'kube-system',
takeOwnership: true,
});
```

### OCI Charts

OCI charts are also supported.
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk-lib/aws-eks/lib/helm-chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ export interface HelmChartOptions {
* @default - CRDs are installed if not already present
*/
readonly skipCrds?: boolean;

/**
* if set, this will use the --take-ownership flag provided since helm >=v3.17.0. Helm will not throw an error if any manifest
* that it renders if already on the cluster and will instead overwrite and annotate it with ownership from the rendered manifest
*
* @default false
*/
readonly takeOwnership?: boolean;
}

/**
Expand Down Expand Up @@ -158,6 +166,8 @@ export class HelmChart extends Construct {
const skipCrds = props.skipCrds ?? false;
// default to set atomic as false
const atomic = props.atomic ?? false;
// default to not take ownership since helm only only added this as of 3.17.0
const takeOwnership = props.takeOwnership ?? false;

this.chartAsset?.grantRead(provider.handlerRole);

Expand All @@ -179,6 +189,7 @@ export class HelmChart extends Construct {
CreateNamespace: createNamespace || undefined,
SkipCrds: skipCrds || undefined,
Atomic: atomic || undefined, // props are stringified so we encode “false” as undefined
TakeOwnership: takeOwnership || undefined,
},
});
}
Expand Down
23 changes: 23 additions & 0 deletions packages/aws-cdk-lib/aws-eks/test/helm-chart.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,29 @@ describe('helm chart', () => {

});

test('should enable takeOwnership operations when specified', () => {
//GIVEN
const { stack, cluster } = testFixtureCluster();

//WHEN
new eks.HelmChart(stack, 'MyAtomicChart', { cluster, chart: 'chart', takeOwnership: true });

//THEN
Template.fromStack(stack).hasResourceProperties(eks.HelmChart.RESOURCE_TYPE, { TakeOwnership: true });
});

test('should disable takeOwnership operations by default', () => {
//GIVEN
const { stack, cluster } = testFixtureCluster();

//WHEN
new eks.HelmChart(stack, 'MyAtomicChart', { cluster, chart: 'chart' });

//THEN
const charts = Template.fromStack(stack).findResources(eks.HelmChart.RESOURCE_TYPE, { Atomic: true });
expect(Object.keys(charts).length).toEqual(0);
});

test('should timeout only after 10 minutes', () => {
// GIVEN
const { stack, cluster } = testFixtureCluster();
Expand Down

0 comments on commit b7dc8eb

Please sign in to comment.