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.

Sign up for GitHub

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

Jump to bottom

fix(eks): k8s resources accidentally deleted due to logical ID change #12053

Merged
merged 1 commit into from
Dec 13, 2020
Merged

fix(eks): k8s resources accidentally deleted due to logical ID change #12053

merged 1 commit into from
Dec 13, 2020

Commits on Dec 13, 2020

  1. fix(eks): k8s resources accidentally deleted due to logical ID change 

    The `KubernetesManifest` construct used `kubectl apply` for both CREATE and UPDATE operations. This means that if, for example, two manifests had resources with the same k8s name (`metadata.name`), the second manifest created will not fail, but rather override the resource definition.

As a consequence, if the logical ID of a `KubernetesManifest` resource was changed (without a change in the physical name), CFN would perform a replacement process which involves a CREATE of the new resource and then a DELETE of the old one. Since the CREATE operation was implemented through `apply`, it succeeded (with no-op) but then the DELETE operation would delete the resource. The result is that the resource was deleted.

The solution is to use `kubectl create --save-config` instead of `kubectl apply` for CREATE operations. This yields the desired CREATE semantics (dah!).

Now, if a `KubernetesManifest` resource is defined with a K8S object name that already exists, the CREATE operation will fail as expected. The logical ID change scenario (resource replacement), would also issue a CREATE operation first which will fail.

To change logical IDs of `KubernetesManifest` resources, users will have to either delete the old resource or change its physical name.

Since this is quite hard to test (due to multi-phase deployments and failure modes), this was tested manually:

1. Defined a manifest with logical name X1 and physical name Y1 -> CREATE was issued
2. Changed logical name to X2 (physical remains Y1) -> update failed because CFN issues a CREATE operation first (#10397)
3. Changed also the physical name to Y2 -> deploy succeeded, new resource created, old resource pruned.

This fixes #10397
    Elad Ben-Israel committed Dec 13, 2020
    Configuration menu
    Copy the full SHA
    e3ea1c1 View commit details
    Browse the repository at this point in the history