Skip to content

Commit

Permalink
fix(action): Protects against current resource conflicts
Browse files Browse the repository at this point in the history
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 <taylor.thomas@microsoft.com>
  • Loading branch information
thomastaylor312 committed Sep 30, 2019
1 parent 0ca3840 commit 36f3a4b
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 14 deletions.
10 changes: 10 additions & 0 deletions pkg/action/install.go
Expand Up @@ -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 {
Expand Down
47 changes: 34 additions & 13 deletions pkg/action/upgrade.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
47 changes: 47 additions & 0 deletions 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
}
2 changes: 1 addition & 1 deletion pkg/kube/client.go
Expand Up @@ -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)
}
Expand Down

0 comments on commit 36f3a4b

Please sign in to comment.