feat(eks): option to disable manifest validation

[ayush987goyal]

Closes #11763

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[ayush987goyal]

For kubctl logs:

Running command: ['kubectl', 'apply', '--kubeconfig', '/tmp/kubeconfig', '-f', '/tmp/manifest.yaml', '--prune', '-l', 'aws.cdk.eks/prune-c8ae759e1d059c6c67a772adfc4e04d8c732118222', '--validate=false']

[ayush987goyal]

@iliapolo @eladb Could you please take a look at this change? It has a big breaking change but I feel this might be okay in a long run since we might have a lot of customizations for the manifests.

[eladb]

Instead of changing the api for addManifest, would it be sufficient to specify this option at the cluster level (same as prune)?

[ayush987goyal]

Instead of changing the api for addManifest, would it be sufficient to specify this option at the cluster level (same as prune)?

That would set the flag for the complete cluster. What about the case when you only want to skip validation for one particular manifest that you added through addManifest?

[eladb]

Instead of changing the api for addManifest, would it be sufficient to specify this option at the cluster level (same as prune)?

That would set the flag for the complete cluster. What about the case when you only want to skip validation for one particular manifest that you added through addManifest?

Do you think that's a very common use case?

You can still just use KubernetesManifest directly.

[ayush987goyal]

You can still just use KubernetesManifest directly.

I am not sure how we can add a manifest intialized via KubernetesManifest to the cluster (apart from addManifest).

[eladb]

I am not sure how we can add a manifest intialized via KubernetesManifest to the cluster (apart from addManifest).

See docs

[ayush987goyal]

Oh understood it now (missed that KubernetesManifest accepts a cluster!).

I will go ahead and make the changes accordingly to keep it only on KubernetesManifest. @eladb Do you think this flag should also be present on the cluster like prune? Where I am slightly concerned on adding it to cluster is to reduce the blast radius (caused by potential validation skip) for all the manifests.

[iliapolo]

@ayush987goyal I think its ok to only expose validate: false on the manifest level and not on the cluster, it stands to reason that this behavior will not normally apply to all manifests in the cluster.

[ayush987goyal]

@iliapolo So I am trying to add an integ tests for this by initilializing KubernetesManifest by providing the cluster.

  private assertManifestWithoutValidation() {
    // apply a kubernetes manifest
    new eks.KubernetesManifest(this, 'HelloAppWithoutValidation', {
      cluster: this.cluster,
      manifest: [{
        apiVersion: 'v1',
        kind: 'Service',
        metadata: { name: 'hello-kubernetes-without-validation' },
        spec: {
          type: 'LoadBalancer',
          ports: [{ port: 80, targetPort: 8080 }],
          selector: { app: 'hello-kubernetes-without-validation' },
        },
      }],
      validate: false,
    });
  }

I am getting the following cfn error:

Failed to create resource. Error: b'Error from server (AlreadyExists): error when creating "/tmp/manifest.yaml": configmaps "aws-auth" already exists\n' Logs: /aws/lambda/aws-cdk-eks-cluster-test-awscdkaws-Handler886CB40B-4P9SW7KB0BEL at invokeUserFunction (/var/task/framework.js:95:19) at process._tickCallback (internal/process/next_tick.js:68:7)

I am thinking if this is related to the scope in which the manifest is created (the context of cluster v/s stack) but not too sure. Do you have any clue on what might be wrong with this?

[iliapolo]

@ayush987goyal Yes, we broke our integration tests in this commit :)

@eladb is working on a fix.

You can build an earlier commit to see your logic works, but probably better just to hold off until the fix is merged since you'll need to re-run the tests.

[ayush987goyal]

Hi @iliapolo , could you please take a look at this now?

[iliapolo]

@ayush987goyal Also notice there is a conflict now... :\

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
• Commit ID: db4bf0a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[ayush987goyal]

@iliapolo Fixed both! :)

[iliapolo]

@ayush987goyal Thanks :)

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).