upstream/helm
1
0
Fork 0
mirror of https://github.com/helm/helm.git synced 2026-04-15 21:59:50 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Merge pull request #30768 from banjoh/em/fix-take-ownership

Browse source 
v3 backport - fix --take-ownership
This commit is contained in:
Scott Rigby 2025-04-21 10:45:45 -04:00 committed by GitHub
commit e4bc4a31b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 365 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
pkg/action/action_test.go
View file

@ -26,6 +26,7 @@ import (
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/kube"
kubefake "helm.sh/helm/v3/pkg/kube/fake"
"helm.sh/helm/v3/pkg/registry"
"helm.sh/helm/v3/pkg/release"
@ -37,6 +38,10 @@ import (
var verbose = flag.Bool("test.log", false, "enable test logging")
func actionConfigFixture(t *testing.T) *Configuration {
return actionConfigFixtureWithDummyResources(t, nil)
}
func actionConfigFixtureWithDummyResources(t *testing.T, dummyResources kube.ResourceList) *Configuration {
t.Helper()
registryClient, err := registry.NewClient()
@ -46,7 +51,7 @@ func actionConfigFixture(t *testing.T) *Configuration {
return &Configuration{
Releases: storage.Init(driver.NewMemory()),
KubeClient: &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}},
KubeClient: &kubefake.FailingKubeClient{PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard}, DummyResources: dummyResources},
Capabilities: chartutil.DefaultCapabilities,
RegistryClient: registryClient,
Log: func(format string, v ...interface{}) {

6
pkg/action/install.go
View file

@ -455,7 +455,11 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource
if len(toBeAdopted) == 0 && len(resources) > 0 {
_, err = i.cfg.KubeClient.Create(resources)
} else if len(resources) > 0 {
_, err = i.cfg.KubeClient.Update(toBeAdopted, resources, i.Force)
if i.TakeOwnership {
_, err = i.cfg.KubeClient.(kube.InterfaceThreeWayMerge).UpdateThreeWayMerge(toBeAdopted, resources, i.Force)
} else {
_, err = i.cfg.KubeClient.Update(toBeAdopted, resources, i.Force)
}
}
if err != nil {
return rel, err

121
pkg/action/install_test.go
View file

@ -21,6 +21,7 @@ import (
"context"
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"regexp"
@ -31,10 +32,19 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kuberuntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/cli-runtime/pkg/resource"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest/fake"
"helm.sh/helm/v3/internal/test"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/kube"
kubefake "helm.sh/helm/v3/pkg/kube/fake"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"
@ -47,6 +57,62 @@ type nameTemplateTestCase struct {
expectedErrorStr string
}
func createDummyResourceList(owned bool) kube.ResourceList {
obj := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "dummyName",
Namespace: "spaced",
},
}
if owned {
obj.Labels = map[string]string{
"app.kubernetes.io/managed-by": "Helm",
}
obj.Annotations = map[string]string{
"meta.helm.sh/release-name": "test-install-release",
"meta.helm.sh/release-namespace": "spaced",
}
}
resInfo := resource.Info{
Name: "dummyName",
Namespace: "spaced",
Mapping: &meta.RESTMapping{
Resource: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployment"},
GroupVersionKind: schema.GroupVersionKind{Group: "apps", Version: "v1", Kind: "Deployment"},
Scope: meta.RESTScopeNamespace,
},
Object: obj,
}
body := io.NopCloser(bytes.NewReader([]byte(kuberuntime.EncodeOrDie(appsv1Codec, obj))))
resInfo.Client = &fake.RESTClient{
GroupVersion: schema.GroupVersion{Group: "apps", Version: "v1"},
NegotiatedSerializer: scheme.Codecs.WithoutConversion(),
Client: fake.CreateHTTPClient(func(_ *http.Request) (*http.Response, error) {
header := http.Header{}
header.Set("Content-Type", kuberuntime.ContentTypeJSON)
return &http.Response{
StatusCode: http.StatusOK,
Header: header,
Body: body,
}, nil
}),
}
var resourceList kube.ResourceList
resourceList.Append(&resInfo)
return resourceList
}
func installActionWithConfig(config *Configuration) *Install {
instAction := NewInstall(config)
instAction.Namespace = "spaced"
instAction.ReleaseName = "test-install-release"
return instAction
}
func installAction(t *testing.T) *Install {
config := actionConfigFixture(t)
instAction := NewInstall(config)
@ -92,6 +158,61 @@ func TestInstallRelease(t *testing.T) {
is.Equal(lastRelease.Info.Status, release.StatusDeployed)
}
func TestInstallReleaseWithTakeOwnership_ResourceNotOwned(t *testing.T) {
// This test will test checking ownership of a resource
// returned by the fake client. If the resource is not
// owned by the chart, ownership is taken.
// To verify ownership has been taken, the fake client
// needs to store state which is a bigger rewrite.
// TODO: Ensure fake kube client stores state. Maybe using
// "k8s.io/client-go/kubernetes/fake" could be sufficient? i.e
// "Client{Namespace: namespace, kubeClient: k8sfake.NewClientset()}"
is := assert.New(t)
// Resource list from cluster is NOT owned by helm chart
config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false))
instAction := installActionWithConfig(config)
instAction.TakeOwnership = true
res, err := instAction.Run(buildChart(), nil)
if err != nil {
t.Fatalf("Failed install: %s", err)
}
rel, err := instAction.cfg.Releases.Get(res.Name, res.Version)
is.NoError(err)
is.Equal(rel.Info.Description, "Install complete")
}
func TestInstallReleaseWithTakeOwnership_ResourceOwned(t *testing.T) {
is := assert.New(t)
// Resource list from cluster is owned by helm chart
config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(true))
instAction := installActionWithConfig(config)
instAction.TakeOwnership = false
res, err := instAction.Run(buildChart(), nil)
if err != nil {
t.Fatalf("Failed install: %s", err)
}
rel, err := instAction.cfg.Releases.Get(res.Name, res.Version)
is.NoError(err)
is.Equal(rel.Info.Description, "Install complete")
}
func TestInstallReleaseWithTakeOwnership_ResourceOwnedNoFlag(t *testing.T) {
is := assert.New(t)
// Resource list from cluster is NOT owned by helm chart
config := actionConfigFixtureWithDummyResources(t, createDummyResourceList(false))
instAction := installActionWithConfig(config)
_, err := instAction.Run(buildChart(), nil)
is.Error(err)
is.Contains(err.Error(), "Unable to continue with install")
}
func TestInstallReleaseWithValues(t *testing.T) {
is := assert.New(t)
instAction := installAction(t)

59
pkg/kube/client.go
View file

@ -46,6 +46,8 @@ import (
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/jsonmergepatch"
"k8s.io/apimachinery/pkg/util/mergepatch"
"k8s.io/apimachinery/pkg/util/strategicpatch"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/cli-runtime/pkg/genericclioptions"
@ -379,14 +381,7 @@ func (c *Client) BuildTable(reader io.Reader, validate bool) (ResourceList, erro
return result, scrubValidationError(err)
}
// Update takes the current list of objects and target list of objects and
// creates resources that don't already exist, updates resources that have been
// modified in the target configuration, and deletes resources from the current
// configuration that are not present in the target configuration. If an error
// occurs, a Result will still be returned with the error, containing all
// resource updates, creations, and deletions that were attempted. These can be
// used for cleanup or other logging purposes.
func (c *Client) Update(original, target ResourceList, force bool) (*Result, error) {
func (c *Client) update(original, target ResourceList, force, threeWayMerge bool) (*Result, error) {
updateErrors := []string{}
res := &Result{}
@ -421,7 +416,7 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err
return errors.Errorf("no %s with the name %q found", kind, info.Name)
}
if err := updateResource(c, info, originalInfo.Object, force); err != nil {
if err := updateResource(c, info, originalInfo.Object, force, threeWayMerge); err != nil {
c.Log("error updating the resource %q:\n\t %v", info.Name, err)
updateErrors = append(updateErrors, err.Error())
}
@ -462,6 +457,31 @@ func (c *Client) Update(original, target ResourceList, force bool) (*Result, err
return res, nil
}
// Update takes the current list of objects and target list of objects and
// creates resources that don't already exist, updates resources that have been
// modified in the target configuration, and deletes resources from the current
// configuration that are not present in the target configuration. If an error
// occurs, a Result will still be returned with the error, containing all
// resource updates, creations, and deletions that were attempted. These can be
// used for cleanup or other logging purposes.
//
// The difference to Update is that UpdateThreeWayMerge does a three-way-merge
// for unstructured objects.
func (c *Client) UpdateThreeWayMerge(original, target ResourceList, force bool) (*Result, error) {
return c.update(original, target, force, true)
}
// Update takes the current list of objects and target list of objects and
// creates resources that don't already exist, updates resources that have been
// modified in the target configuration, and deletes resources from the current
// configuration that are not present in the target configuration. If an error
// occurs, a Result will still be returned with the error, containing all
// resource updates, creations, and deletions that were attempted. These can be
// used for cleanup or other logging purposes.
func (c *Client) Update(original, target ResourceList, force bool) (*Result, error) {
return c.update(original, target, force, false)
}
// Delete deletes Kubernetes resources specified in the resources list with
// background cascade deletion. It will attempt to delete all resources even
// if one or more fail and collect any errors. All successfully deleted items
@ -617,7 +637,7 @@ func deleteResource(info *resource.Info, policy metav1.DeletionPropagation) erro
})
}
func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.PatchType, error) {
func createPatch(target *resource.Info, current runtime.Object, threeWayMergeForUnstructured bool) ([]byte, types.PatchType, error) {
oldData, err := json.Marshal(current)
if err != nil {
return nil, types.StrategicMergePatchType, errors.Wrap(err, "serializing current configuration")
@ -645,7 +665,7 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
// Unstructured objects, such as CRDs, may not have a not registered error
// returned from ConvertToVersion. Anything that's unstructured should
// use the jsonpatch.CreateMergePatch. Strategic Merge Patch is not supported
// use generic JSON merge patch. Strategic Merge Patch is not supported
// on objects like CRDs.
_, isUnstructured := versionedObject.(runtime.Unstructured)
@ -653,6 +673,19 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
_, isCRD := versionedObject.(*apiextv1beta1.CustomResourceDefinition)
if isUnstructured || isCRD {
if threeWayMergeForUnstructured {
// from https://github.com/kubernetes/kubectl/blob/b83b2ec7d15f286720bccf7872b5c72372cb8e80/pkg/cmd/apply/patcher.go#L129
preconditions := []mergepatch.PreconditionFunc{
mergepatch.RequireKeyUnchanged("apiVersion"),
mergepatch.RequireKeyUnchanged("kind"),
mergepatch.RequireMetadataKeyUnchanged("name"),
}
patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(oldData, newData, currentData, preconditions...)
if err != nil && mergepatch.IsPreconditionFailed(err) {
err = fmt.Errorf("%w: at least one field was changed: apiVersion, kind or name", err)
}
return patch, types.MergePatchType, err
}
// fall back to generic JSON merge patch
patch, err := jsonpatch.CreateMergePatch(oldData, newData)
return patch, types.MergePatchType, err
@ -667,7 +700,7 @@ func createPatch(target *resource.Info, current runtime.Object) ([]byte, types.P
return patch, types.StrategicMergePatchType, err
}
func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, force bool) error {
func updateResource(c *Client, target *resource.Info, currentObj runtime.Object, force, threeWayMergeForUnstructured bool) error {
var (
obj runtime.Object
helper = resource.NewHelper(target.Client, target.Mapping).WithFieldManager(getManagedFieldsManager())
@ -683,7 +716,7 @@ func updateResource(c *Client, target *resource.Info, currentObj runtime.Object,
}
c.Log("Replaced %q with kind %s for kind %s", target.Name, currentObj.GetObjectKind().GroupVersionKind().Kind, kind)
} else {
patch, patchType, err := createPatch(target, currentObj)
patch, patchType, err := createPatch(target, currentObj, threeWayMergeForUnstructured)
if err != nil {
return errors.Wrap(err, "failed to create patch")
}

169
pkg/kube/client_test.go
View file

@ -27,8 +27,13 @@ import (
"github.com/stretchr/testify/assert"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
jsonserializer "k8s.io/apimachinery/pkg/runtime/serializer/json"
"k8s.io/apimachinery/pkg/types"
"k8s.io/cli-runtime/pkg/resource"
k8sfake "k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"
@ -208,7 +213,7 @@ func TestCreate(t *testing.T) {
})
}
func TestUpdate(t *testing.T) {
func testUpdate(t *testing.T, threeWayMerge bool) {
listA := newPodList("starfish", "otter", "squid")
listB := newPodList("starfish", "otter", "dolphin")
listC := newPodList("starfish", "otter", "dolphin")
@ -279,7 +284,12 @@ func TestUpdate(t *testing.T) {
t.Fatal(err)
}
result, err := c.Update(first, second, false)
var result *Result
if threeWayMerge {
result, err = c.UpdateThreeWayMerge(first, second, false)
} else {
result, err = c.Update(first, second, false)
}
if err != nil {
t.Fatal(err)
}
@ -328,6 +338,14 @@ func TestUpdate(t *testing.T) {
}
}
func TestUpdate(t *testing.T) {
testUpdate(t, false)
}
func TestUpdateThreeWayMerge(t *testing.T) {
testUpdate(t, true)
}
func TestBuild(t *testing.T) {
tests := []struct {
name string
@ -898,3 +916,150 @@ spec:
var resourceQuotaConflict = []byte(`
{"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Operation cannot be fulfilled on resourcequotas \"quota\": the object has been modified; please apply your changes to the latest version and try again","reason":"Conflict","details":{"name":"quota","kind":"resourcequotas"},"code":409}`)
type createPatchTestCase struct {
name string
// The target state.
target *unstructured.Unstructured
// The current state as it exists in the release.
current *unstructured.Unstructured
// The actual state as it exists in the cluster.
actual *unstructured.Unstructured
threeWayMergeForUnstructured bool
// The patch is supposed to transfer the current state to the target state,
// thereby preserving the actual state, wherever possible.
expectedPatch string
expectedPatchType types.PatchType
}
func (c createPatchTestCase) run(t *testing.T) {
scheme := runtime.NewScheme()
v1.AddToScheme(scheme)
encoder := jsonserializer.NewSerializerWithOptions(
jsonserializer.DefaultMetaFactory, scheme, scheme, jsonserializer.SerializerOptions{
Yaml: false, Pretty: false, Strict: true,
},
)
objBody := func(obj runtime.Object) io.ReadCloser {
return io.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(encoder, obj))))
}
header := make(http.Header)
header.Set("Content-Type", runtime.ContentTypeJSON)
restClient := &fake.RESTClient{
NegotiatedSerializer: unstructuredSerializer,
Resp: &http.Response{
StatusCode: 200,
Body: objBody(c.actual),
Header: header,
},
}
targetInfo := &resource.Info{
Client: restClient,
Namespace: "default",
Name: "test-obj",
Object: c.target,
Mapping: &meta.RESTMapping{
Resource: schema.GroupVersionResource{
Group: "crd.com",
Version: "v1",
Resource: "datas",
},
Scope: meta.RESTScopeNamespace,
},
}
patch, patchType, err := createPatch(targetInfo, c.current, c.threeWayMergeForUnstructured)
if err != nil {
t.Fatalf("Failed to create patch: %v", err)
}
if c.expectedPatch != string(patch) {
t.Errorf("Unexpected patch.\nTarget:\n%s\nCurrent:\n%s\nActual:\n%s\n\nExpected:\n%s\nGot:\n%s",
c.target,
c.current,
c.actual,
c.expectedPatch,
string(patch),
)
}
if patchType != types.MergePatchType {
t.Errorf("Expected patch type %s, got %s", types.MergePatchType, patchType)
}
}
func newTestCustomResourceData(metadata map[string]string, spec map[string]interface{}) *unstructured.Unstructured {
if metadata == nil {
metadata = make(map[string]string)
}
if _, ok := metadata["name"]; !ok {
metadata["name"] = "test-obj"
}
if _, ok := metadata["namespace"]; !ok {
metadata["namespace"] = "default"
}
o := map[string]interface{}{
"apiVersion": "crd.com/v1",
"kind": "Data",
"metadata": metadata,
}
if len(spec) > 0 {
o["spec"] = spec
}
return &unstructured.Unstructured{
Object: o,
}
}
func TestCreatePatchCustomResourceMetadata(t *testing.T) {
target := newTestCustomResourceData(map[string]string{
"meta.helm.sh/release-name": "foo-simple",
"meta.helm.sh/release-namespace": "default",
"objectset.rio.cattle.io/id": "default-foo-simple",
}, nil)
testCase := createPatchTestCase{
name: "take ownership of resource",
target: target,
current: target,
actual: newTestCustomResourceData(nil, map[string]interface{}{
"color": "red",
}),
threeWayMergeForUnstructured: true,
expectedPatch: `{"metadata":{"meta.helm.sh/release-name":"foo-simple","meta.helm.sh/release-namespace":"default","objectset.rio.cattle.io/id":"default-foo-simple"}}`,
expectedPatchType: types.MergePatchType,
}
t.Run(testCase.name, testCase.run)
// Previous behavior.
testCase.threeWayMergeForUnstructured = false
testCase.expectedPatch = `{}`
t.Run(testCase.name, testCase.run)
}
func TestCreatePatchCustomResourceSpec(t *testing.T) {
target := newTestCustomResourceData(nil, map[string]interface{}{
"color": "red",
"size": "large",
})
testCase := createPatchTestCase{
name: "merge with spec of existing custom resource",
target: target,
current: target,
actual: newTestCustomResourceData(nil, map[string]interface{}{
"color": "red",
"weight": "heavy",
}),
threeWayMergeForUnstructured: true,
expectedPatch: `{"spec":{"size":"large"}}`,
expectedPatchType: types.MergePatchType,
}
t.Run(testCase.name, testCase.run)
// Previous behavior.
testCase.threeWayMergeForUnstructured = false
testCase.expectedPatch = `{}`
t.Run(testCase.name, testCase.run)
}

12
pkg/kube/fake/fake.go
View file

@ -44,6 +44,7 @@ type FailingKubeClient struct {
BuildError error
BuildTableError error
BuildDummy bool
DummyResources kube.ResourceList
BuildUnstructuredError error
WaitAndGetCompletedPodPhaseError error
WaitDuration time.Duration
@ -114,11 +115,22 @@ func (f *FailingKubeClient) Update(r, modified kube.ResourceList, ignoreMe bool)
return f.PrintingKubeClient.Update(r, modified, ignoreMe)
}
// Update returns the configured error if set or prints
func (f *FailingKubeClient) UpdateThreeWayMerge(r, modified kube.ResourceList, ignoreMe bool) (*kube.Result, error) {
if f.UpdateError != nil {
return &kube.Result{}, f.UpdateError
}
return f.PrintingKubeClient.Update(r, modified, ignoreMe)
}
// Build returns the configured error if set or prints
func (f *FailingKubeClient) Build(r io.Reader, _ bool) (kube.ResourceList, error) {
if f.BuildError != nil {
return []*resource.Info{}, f.BuildError
}
if f.DummyResources != nil {
return f.DummyResources, nil
}
if f.BuildDummy {
return createDummyResourceList(), nil
}

8
pkg/kube/interface.go
View file

@ -80,6 +80,13 @@ type InterfaceExt interface {
WaitForDelete(resources ResourceList, timeout time.Duration) error
}
// InterfaceThreeWayMerge was introduced to avoid breaking backwards compatibility for Interface implementers.
//
// TODO Helm 4: Remove InterfaceThreeWayMerge and integrate its method(s) into the Interface.
type InterfaceThreeWayMerge interface {
UpdateThreeWayMerge(original, target ResourceList, force bool) (*Result, error)
}
// InterfaceLogs was introduced to avoid breaking backwards compatibility for Interface implementers.
//
// TODO Helm 4: Remove InterfaceLogs and integrate its method(s) into the Interface.
@ -123,6 +130,7 @@ type InterfaceResources interface {
var _ Interface = (*Client)(nil)
var _ InterfaceExt = (*Client)(nil)
var _ InterfaceThreeWayMerge = (*Client)(nil)
var _ InterfaceLogs = (*Client)(nil)
var _ InterfaceDeletionPropagation = (*Client)(nil)
var _ InterfaceResources = (*Client)(nil)