From 36f3a4b326f1df1ad50cfa19c47808a7e42bd294 Mon Sep 17 00:00:00 2001 From: Taylor Thomas Date: Mon, 30 Sep 2019 16:05:41 -0600 Subject: [PATCH] fix(action): Protects against current resource conflicts Currently, if using the --atomic flag or deleting a release that failed due to an already existing resource, Helm will deleting those resources that aren't managed by it. This PR fixes the issue by checking for pre-existing resources during install and upgrade. This is done as a validation step so the release will not even be started if resources currently exist. This PR is inspired by @xchapter7x's work in #3477. This also fixes a small bug in upgrade where deletes fail if the resource was already deletes Fixes #6407 Signed-off-by: Taylor Thomas --- pkg/action/install.go | 10 +++++++++ pkg/action/upgrade.go | 47 ++++++++++++++++++++++++++++++------------ pkg/action/validate.go | 47 ++++++++++++++++++++++++++++++++++++++++++ pkg/kube/client.go | 2 +- 4 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 pkg/action/validate.go diff --git a/pkg/action/install.go b/pkg/action/install.go index c2dce1817..f14875368 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -231,6 +231,16 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release. return rel, nil } + // Install requires an extra validation step of checking that resources + // don't already exist before we actually create resources. If we continue + // forward and create the release object with resources that already exist, + // we'll end up in a state where we will delete those resources upon + // deleting the release because the manifest will be pointing at that + // resource + if err := existingResourceConflict(resources); err != nil { + return nil, errors.Wrap(err, "rendered manifests contain a resource that already exists. Unable to continue with install") + } + // If Replace is true, we need to supercede the last release. if i.Replace { if err := i.replaceRelease(rel); err != nil { diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 6046026dd..91d31b95c 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/cli-runtime/pkg/resource" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chartutil" @@ -88,13 +89,6 @@ func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface u.cfg.Releases.MaxHistory = u.MaxHistory - if !u.DryRun { - u.cfg.Log("creating upgraded release for %s", name) - if err := u.cfg.Releases.Create(upgradedRelease); err != nil { - return nil, err - } - } - u.cfg.Log("performing update for %s", name) res, err := u.performUpgrade(currentRelease, upgradedRelease) if err != nil { @@ -196,12 +190,6 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin } func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Release) (*release.Release, error) { - if u.DryRun { - u.cfg.Log("dry run for %s", upgradedRelease.Name) - upgradedRelease.Info.Description = "Dry run complete" - return upgradedRelease, nil - } - current, err := u.cfg.KubeClient.Build(bytes.NewBufferString(originalRelease.Manifest)) if err != nil { return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from current release manifest") @@ -211,6 +199,34 @@ func (u *Upgrade) performUpgrade(originalRelease, upgradedRelease *release.Relea return upgradedRelease, errors.Wrap(err, "unable to build kubernetes objects from new release manifest") } + if u.DryRun { + u.cfg.Log("dry run for %s", upgradedRelease.Name) + upgradedRelease.Info.Description = "Dry run complete" + return upgradedRelease, nil + } + + // Do a basic diff using gvk + name to figure out what new resources are being created so we can validate they don't already exist + existingResources := make(map[string]bool) + for _, r := range current { + existingResources[objectKey(r)] = true + } + + var toBeCreated kube.ResourceList + for _, r := range target { + if !existingResources[objectKey(r)] { + toBeCreated = append(toBeCreated, r) + } + } + + if err := existingResourceConflict(toBeCreated); err != nil { + return nil, errors.Wrap(err, "rendered manifests contain a new resource that already exists. Unable to continue with update") + } + + u.cfg.Log("creating upgraded release for %s", upgradedRelease.Name) + if err := u.cfg.Releases.Create(upgradedRelease); err != nil { + return nil, err + } + // pre-upgrade hooks if !u.DisableHooks { if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.Timeout); err != nil { @@ -396,3 +412,8 @@ func recreate(cfg *Configuration, resources kube.ResourceList) error { } return nil } + +func objectKey(r *resource.Info) string { + gvk := r.Object.GetObjectKind().GroupVersionKind() + return fmt.Sprintf("%s/%s/%s", gvk.GroupVersion().String(), gvk.Kind, r.Name) +} diff --git a/pkg/action/validate.go b/pkg/action/validate.go new file mode 100644 index 000000000..56b0acdeb --- /dev/null +++ b/pkg/action/validate.go @@ -0,0 +1,47 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "fmt" + + "github.com/pkg/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/cli-runtime/pkg/resource" + + "helm.sh/helm/pkg/kube" +) + +func existingResourceConflict(resources kube.ResourceList) error { + err := resources.Visit(func(info *resource.Info, err error) error { + if err != nil { + return err + } + + helper := resource.NewHelper(info.Client, info.Mapping) + if _, err := helper.Get(info.Namespace, info.Name, info.Export); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + + return errors.Wrap(err, "could not get information about the resource") + } + + return fmt.Errorf("existing resource conflict: kind: %s, namespace: %s, name: %s", info.Mapping.GroupVersionKind.Kind, info.Namespace, info.Name) + }) + return err +} diff --git a/pkg/kube/client.go b/pkg/kube/client.go index b051a508b..3c53801e2 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -191,7 +191,7 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err for _, info := range original.Difference(target) { c.Log("Deleting %q in %s...", info.Name, info.Namespace) res.Deleted = append(res.Deleted, info) - if err := deleteResource(info); err != nil { + if err := deleteResource(info); err != nil && !apierrors.IsNotFound(err) { c.Log("Failed to delete %q, err: %s", info.Name, err) return res, errors.Wrapf(err, "Failed to delete %q", info.Name) }