Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[aws-eks] Managed NodeGroup disk encryption #9006

Open
2 tasks
DarkmatterVale opened this issue Jul 10, 2020 · 15 comments
Open
2 tasks

[aws-eks] Managed NodeGroup disk encryption #9006

DarkmatterVale opened this issue Jul 10, 2020 · 15 comments
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1

Comments

@DarkmatterVale
Copy link

Encrypt EBS volumes backing Managed NodeGroup EC2 instances.

Use Case

We want to enforce security best practices when using EKS Managed NodeGroups.

Proposed Solution

Add a boolean parameter "diskEncrypted" in managed-nodegroup.

Requires EKS NodeGroup CFN to add a boolean encryption parameter as well.

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@DarkmatterVale DarkmatterVale added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2020
@SomayaB SomayaB changed the title eks: Managed NodeGroup disk encryption [eks] Managed NodeGroup disk encryption Jul 10, 2020
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Jul 10, 2020
@eladb
Copy link
Contributor

eladb commented Jul 16, 2020

We won't be able to support this until CFN support is added (copy @tabern)

@eladb eladb added the effort/small Small work item – less than a day of effort label Jul 16, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Jul 17, 2020
@eladb eladb added this to the EKS Dev Preview milestone Jul 22, 2020
@eladb eladb assigned iliapolo and unassigned eladb Aug 4, 2020
@iliapolo iliapolo removed this from the EKS Dev Preview milestone Aug 11, 2020
@iliapolo iliapolo changed the title [eks] Managed NodeGroup disk encryption [aws-eks] Managed NodeGroup disk encryption Aug 16, 2020
@iliapolo iliapolo added the p1 label Aug 29, 2020
@alanprot
Copy link

alanprot commented Sep 9, 2020

I guess we can achieve the EBS encryption using LaunchTemplate?

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-launchtemplatedata.html

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-blockdevicemapping-ebs.html

But seems that CDK also does not allow to specify a launch template for the node group.

@iliapolo
Copy link
Contributor

@alanprot Actually launch template support was just added: #9881

Should be available in the next release.

@shrivastavshubham34
Copy link

@iliapolo I don't see the encryption option below, is this a work in progress?
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-ec2.EbsDeviceProps.html

@iliapolo
Copy link
Contributor

iliapolo commented Mar 8, 2021

@shrivastavshubham34 I was referring to the addition of launch template support in EKS. You can achieve the encryption by specifying it in the launch template using the ec2.CfnLaunchTemplate construct.

I'm not sure exactly what the EbsDeviceProps you link are referring to in this context.

@shrivastavshubham34
Copy link

Thanks, ec2.LaunchTemplate has argument called blockDevices which had BlockDeviceVolume, but i'll try CfnLaunchTemplate

@shrivastavshubham34
Copy link

@iliapolo I keep getting "Instance types must be specified within the launch template".
I already have a construct specifying this, but it won't accept. Is there a workaround?

        lt = ec2.CfnLaunchTemplate(self, "LaunchTemplate",
            launch_template_data={
                # "instance_type": CFG['eks']['ngIntel']['instance_type'],
                "block_device_mappings": {
                    "ebs": {
                        "encrypted": "true"
                    }
                }
            }
        )

        ng1 = eks_cluster.add_nodegroup_capacity('ng'+CFG['eks']['ngIntel']['type']+CFG['environment_name'],
        ami_type=eks.NodegroupAmiType.AL2_X86_64, # AL2_ARM_64, change for ARM
        launch_template_spec={
            "id": lt.ref,
            "version": lt.attr_latest_version_number
        },
        remote_access=eks.NodegroupRemoteAccess(ssh_key_name=CFG['eks']['ngIntel']['key_pair'],source_security_groups=[remote_access_sg]), 

        instance_type=ec2.InstanceType(CFG['eks']['ngIntel']['instance_type']),
        subnets=ec2.SubnetSelection(subnets=[s for s in vpc.private_subnets if s.subnet_id in CFG['subnets']['private']]),
        min_size=CFG['eks']['ngIntel']['min_size'],
        max_size=CFG['eks']['ngIntel']['max_size']
        )

@iliapolo
Copy link
Contributor

iliapolo commented Mar 9, 2021

@shrivastavshubham34 Are you getting this at deploy time? Can you share the node group section of the synthesized CloudFormation template?

@shrivastavshubham34
Copy link

Sure, @iliapolo
Current nodegroup configuration

  EKSClusterdevNodegroupnginteldevC0164185:
    Type: AWS::EKS::Nodegroup
    Properties:
      ClusterName:
        Ref: EKSClusterdev8A9BE0DD
      NodeRole:
        Fn::GetAtt:
          - EKSClusterdevNodegroupnginteldevNodeGroupRoleB0F8FD70
          - Arn
      Subnets:
        - subnet-xxxxxxxxxxxxxxxxxxx
        - subnet-xxxxxxxxxxxxxxxxxxx
      AmiType: AL2_x86_64
      ForceUpdateEnabled: true
      InstanceTypes:
        - m5.large
      RemoteAccess:
        Ec2SshKey: key1
        SourceSecurityGroups:
          - sg-xxxxxxxxxxxxxxxxx
      ScalingConfig:
        DesiredSize: 3
        MaxSize: 5
        MinSize: 3
    Metadata:
      aws:cdk:path: microeks-cdk-dev/EKS_Cluster_dev/Nodegroupnginteldev/Resource

Nodegroup configuration after I add LaunchTemplate:

  EKSClusterdevNodegroupnginteldevC0164185:
    Type: AWS::EKS::Nodegroup
    Properties:
      ClusterName:
        Ref: EKSClusterdev8A9BE0DD
      NodeRole:
        Fn::GetAtt:
          - EKSClusterdevNodegroupnginteldevNodeGroupRoleB0F8FD70
          - Arn
      Subnets:
        - subnet-xxxxxxxxxxxxxxxxxxx
        - subnet-xxxxxxxxxxxxxxxxxxx
      AmiType: AL2_x86_64
      ForceUpdateEnabled: true
      LaunchTemplate:
        Id:
          Ref: LaunchTemplate
        Version:
          Fn::GetAtt:
            - LaunchTemplate
            - LatestVersionNumber
      RemoteAccess:
        Ec2SshKey: key1
        SourceSecurityGroups:
          - sg-xxxxxxxxxxxxxxxxxxx
      ScalingConfig:
        DesiredSize: 3
        MaxSize: 5
        MinSize: 3
    Metadata:
      aws:cdk:path: microeks-cdk-dev/EKS_Cluster_dev/Nodegroupnginteldev/Resource

@iliapolo
Copy link
Contributor

@shrivastavshubham34 What stands out from the template after you add the launch template is that its missing the instance types declaration from before:

InstanceTypes:
  - m5.large

This means that instance types are not defined nor in the node group config, nor in the launch template. So the error comes up.
Are you by chance removing the instance_type property from the config when you add the launch template?

Also note that your CfnLaunchTemplate is not correctly configured. It should use strong types, not dictionaries:

        lt = ec2.CfnLaunchTemplate(self, "LaunchTemplate",
            launch_template_data=ec2.CfnLaunchTemplate.LaunchTemplateDataProperty(
              block_device_mappings=[
                ec2.CfnLaunchTemplate.BlockDeviceMappingProperty(
                  ebs=ec2.CfnLaunchTemplate.EbsProperty(
                    encrypted=True))],
              instance_type="m5.large"))

If you have any additional questions/problems i'll ask you to please open a dedicated issue so we keep this issue clean - we have already diverged quite a bit :)

Thanks

@shrivastavshubham34
Copy link

@iliapolo sorry for the delayed response, I was able to fix the issue. And yes I did not remove the instance_type property from the config, it just doesn't accept it. I had to add the instance_type property to launch the template and recreate the node group to make it work, but it's okay since it spins up a new one before deleting the old NG.

        ng_encrypted_lt = ec2.CfnLaunchTemplate(self, "NGEncryptLaunchTemplate",
            launch_template_data=ec2.CfnLaunchTemplate.LaunchTemplateDataProperty(
                instance_type=CFG['eks']['ngIntel']['instance_type'],
                key_name=CFG['eks']['ngIntel']['key_pair'],
                block_device_mappings=[ec2.CfnLaunchTemplate.BlockDeviceMappingProperty(
                    device_name="/dev/xvda",
                    ebs=ec2.CfnLaunchTemplate.EbsProperty(
                        volume_type="gp2",
                        volume_size=20,
                        encrypted=True
                    )
                )]
            )
        )

@iliapolo iliapolo removed their assignment Jun 27, 2021
@maeghan-porter
Copy link

maeghan-porter commented Dec 9, 2022

My use case where I ran in to needing this is I have an existing cluster I am importing and when I create a nodegroup for it, without a launch template, the nodes come up great and connect to the cluster just fine, but they are lacking block device encryption, which we want for security hygiene purposes. However if I add a launch template where I'm just defining the block device and setting it to encrypted, the nodes that come up in that format can no longer connect to the cluster. I'm really not sure why this is. But as a result I cannot use the launch template and so cannot set the block device to be encrypted.
Given that, I figure if I could set the block devices to be encrypted via the nodegroup definition, in theory, whatever is happening in the background that gives my nodegroups access to the cluster should work.

@pahud
Copy link
Contributor

pahud commented Jan 25, 2023

Still relevant. I will look into this and see if there's anything we could do to work around this.

@iliapolo
Copy link
Contributor

iliapolo commented May 16, 2023

It sounds the solution for this is 3 fold:

  1. Add support for ebs encryption in the ec2.LaunchTemplate construct.
  2. Make the Nodegroup construct use the ec2.LaunchTemplate by default.
  3. Possibly expose an encrypted property on the Nodegroup construct to make it easier to create default encrypted volumes.

Putting this here for anyone looking interested in contributing. We still haven't put this on our roadmap. Stay tuned.

@pahud pahud self-assigned this May 24, 2024
@pahud
Copy link
Contributor

pahud commented May 24, 2024

Yep we should support ebs encryption for the ec2.LaunchTemplate construct.

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-ebs.html#cfn-ec2-launchtemplate-ebs-encrypted

related to #6459

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

8 participants