pull/32680/merge
Weiyi Li 2 days ago committed by GitHub
commit fa928ca35e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,223 @@
/*
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 driver
import (
"bytes"
"maps"
"slices"
"k8s.io/apimachinery/pkg/runtime"
chart "helm.sh/helm/v4/internal/chart/v3"
rspb "helm.sh/helm/v4/internal/release/v2"
"helm.sh/helm/v4/pkg/chart/common"
)
// copyRelease returns a deep copy of rls.
//
// The persistent drivers serialize releases on write and deserialize them on
// read, so callers can never reach the stored data. The memory driver keeps
// releases as Go values, and therefore has to copy them explicitly to offer the
// same isolation.
//
// Values held in the release that Helm does not model, such as custom types
// stored in Config, are copied as interface values rather than recursively.
func copyRelease(rls *rspb.Release) *rspb.Release {
if rls == nil {
return nil
}
return &rspb.Release{
Name: rls.Name,
Info: copyInfo(rls.Info),
Chart: copyChart(rls.Chart),
Config: copyValues(rls.Config),
Manifest: rls.Manifest,
Hooks: copyHooks(rls.Hooks),
Version: rls.Version,
Namespace: rls.Namespace,
Labels: maps.Clone(rls.Labels),
ApplyMethod: rls.ApplyMethod,
}
}
func copyInfo(info *rspb.Info) *rspb.Info {
if info == nil {
return nil
}
out := *info
if info.Resources != nil {
out.Resources = make(map[string][]runtime.Object, len(info.Resources))
for name, objs := range info.Resources {
copied := make([]runtime.Object, len(objs))
for i, obj := range objs {
if obj != nil {
copied[i] = obj.DeepCopyObject()
}
}
out.Resources[name] = copied
}
}
return &out
}
func copyHooks(hooks []*rspb.Hook) []*rspb.Hook {
if hooks == nil {
return nil
}
out := make([]*rspb.Hook, len(hooks))
for i, hook := range hooks {
if hook == nil {
continue
}
copied := *hook
copied.Events = slices.Clone(hook.Events)
copied.DeletePolicies = slices.Clone(hook.DeletePolicies)
copied.OutputLogPolicies = slices.Clone(hook.OutputLogPolicies)
out[i] = &copied
}
return out
}
func copyChart(ch *chart.Chart) *chart.Chart {
if ch == nil {
return nil
}
out := &chart.Chart{
Raw: copyFiles(ch.Raw),
Metadata: copyMetadata(ch.Metadata),
Lock: copyLock(ch.Lock),
Templates: copyFiles(ch.Templates),
Values: copyValues(ch.Values),
Schema: bytes.Clone(ch.Schema),
SchemaModTime: ch.SchemaModTime,
Files: copyFiles(ch.Files),
ModTime: ch.ModTime,
}
// AddDependency sets the parent of each dependency, which rebuilds the
// chart tree without reaching into the unexported fields.
for _, dep := range ch.Dependencies() {
out.AddDependency(copyChart(dep))
}
return out
}
func copyMetadata(md *chart.Metadata) *chart.Metadata {
if md == nil {
return nil
}
out := *md
out.Sources = slices.Clone(md.Sources)
out.Keywords = slices.Clone(md.Keywords)
out.Annotations = maps.Clone(md.Annotations)
if md.Maintainers != nil {
out.Maintainers = make([]*chart.Maintainer, len(md.Maintainers))
for i, m := range md.Maintainers {
if m == nil {
continue
}
copied := *m
out.Maintainers[i] = &copied
}
}
out.Dependencies = copyDependencies(md.Dependencies)
return &out
}
func copyLock(lock *chart.Lock) *chart.Lock {
if lock == nil {
return nil
}
out := *lock
out.Dependencies = copyDependencies(lock.Dependencies)
return &out
}
func copyDependencies(deps []*chart.Dependency) []*chart.Dependency {
if deps == nil {
return nil
}
out := make([]*chart.Dependency, len(deps))
for i, dep := range deps {
if dep == nil {
continue
}
copied := *dep
copied.Tags = slices.Clone(dep.Tags)
copied.ImportValues = copyValueSlice(dep.ImportValues)
out[i] = &copied
}
return out
}
func copyFiles(files []*common.File) []*common.File {
if files == nil {
return nil
}
out := make([]*common.File, len(files))
for i, file := range files {
if file == nil {
continue
}
copied := *file
copied.Data = bytes.Clone(file.Data)
out[i] = &copied
}
return out
}
// copyValues deep copies the maps and slices that make up decoded chart values
// and release config. Other values are copied as interface values, which is
// enough for the scalars these maps normally hold.
func copyValues(values map[string]any) map[string]any {
if values == nil {
return nil
}
out := make(map[string]any, len(values))
for key, value := range values {
out[key] = copyValue(value)
}
return out
}
func copyValue(value any) any {
switch typed := value.(type) {
case map[string]any:
return copyValues(typed)
case map[any]any:
out := make(map[any]any, len(typed))
for key, nested := range typed {
out[key] = copyValue(nested)
}
return out
case []any:
return copyValueSlice(typed)
default:
return value
}
}
func copyValueSlice(values []any) []any {
if values == nil {
return nil
}
out := make([]any, len(values))
for i, value := range values {
out[i] = copyValue(value)
}
return out
}

@ -0,0 +1,114 @@
/*
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 driver
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
chart "helm.sh/helm/v4/internal/chart/v3"
rspb "helm.sh/helm/v4/internal/release/v2"
"helm.sh/helm/v4/pkg/chart/common"
rcommon "helm.sh/helm/v4/pkg/release/common"
)
// chartFixture builds a chart with a dependency, raw files and nested values so
// the copy can be checked against the parts a JSON round-trip would drop.
func chartFixture() *chart.Chart {
ch := &chart.Chart{
Metadata: &chart.Metadata{
Name: "parent",
Annotations: map[string]string{"example.com/team": "helm"},
Maintainers: []*chart.Maintainer{{Name: "maintainer"}},
},
Raw: []*common.File{{Name: "Chart.yaml", Data: []byte("name: parent")}},
Templates: []*common.File{{Name: "templates/cm.yaml", Data: []byte("kind: ConfigMap")}},
Values: map[string]any{"nested": map[string]any{"replicas": 1}},
Schema: []byte("schema-bytes"),
}
ch.AddDependency(&chart.Chart{Metadata: &chart.Metadata{Name: "child"}})
return ch
}
func releaseFixture() *rspb.Release {
rls := releaseStub("copy-me", 1, "default", rcommon.StatusDeployed)
rls.Chart = chartFixture()
rls.Config = map[string]any{"nested": map[string]any{"enabled": true}}
rls.Hooks = []*rspb.Hook{{
Name: "pre-install",
Events: []rspb.HookEvent{rspb.HookPreInstall},
}}
return rls
}
func TestCopyReleaseIsIndependent(t *testing.T) {
rls := releaseFixture()
copied := copyRelease(rls)
require.NotSame(t, rls, copied)
assert.Equal(t, rls, copied, "a copy should hold the same data as the original")
copied.Info.Status = rcommon.StatusFailed
copied.Labels["key1"] = "changed"
copied.Config["nested"].(map[string]any)["enabled"] = false
copied.Chart.Metadata.Name = "renamed"
copied.Chart.Metadata.Annotations["example.com/team"] = "changed"
copied.Chart.Metadata.Maintainers[0].Name = "changed"
copied.Chart.Values["nested"].(map[string]any)["replicas"] = 2
copied.Chart.Raw[0].Data[0] = 'X'
copied.Chart.Schema[0] = 'X'
copied.Chart.Dependencies()[0].Metadata.Name = "changed"
copied.Hooks[0].Events[0] = rspb.HookPostInstall
assert.Equal(t, rcommon.StatusDeployed, rls.Info.Status)
assert.Equal(t, "val1", rls.Labels["key1"])
assert.Equal(t, true, rls.Config["nested"].(map[string]any)["enabled"])
assert.Equal(t, "parent", rls.Chart.Metadata.Name)
assert.Equal(t, "helm", rls.Chart.Metadata.Annotations["example.com/team"])
assert.Equal(t, "maintainer", rls.Chart.Metadata.Maintainers[0].Name)
assert.Equal(t, 1, rls.Chart.Values["nested"].(map[string]any)["replicas"])
assert.Equal(t, []byte("name: parent"), rls.Chart.Raw[0].Data)
assert.Equal(t, []byte("schema-bytes"), rls.Chart.Schema)
assert.Equal(t, "child", rls.Chart.Dependencies()[0].Metadata.Name)
assert.Equal(t, rspb.HookPreInstall, rls.Hooks[0].Events[0])
}
// The chart tree is rebuilt through AddDependency, so the copied dependency has
// to point at the copied parent rather than the original one.
func TestCopyReleaseRebuildsChartTree(t *testing.T) {
copied := copyRelease(releaseFixture())
require.Len(t, copied.Chart.Dependencies(), 1)
dep := copied.Chart.Dependencies()[0]
assert.Same(t, copied.Chart, dep.Parent())
assert.True(t, copied.Chart.IsRoot())
assert.Equal(t, "parent.child", dep.ChartPath())
}
func TestCopyReleaseKeepsNilFields(t *testing.T) {
copied := copyRelease(&rspb.Release{Name: "sparse"})
assert.Equal(t, "sparse", copied.Name)
assert.Nil(t, copied.Info)
assert.Nil(t, copied.Chart)
assert.Nil(t, copied.Config)
assert.Nil(t, copied.Hooks)
assert.Nil(t, copied.Labels)
assert.Nil(t, copyRelease(nil))
}

@ -79,7 +79,7 @@ func (mem *Memory) Get(key string) (release.Releaser, error) {
}
if recs, ok := mem.cache[mem.namespace][name]; ok {
if r := recs.Get(key); r != nil {
return r.rls, nil
return copyRelease(r.rls), nil
}
}
return nil, ErrReleaseNotFound
@ -100,8 +100,11 @@ func (mem *Memory) List(filter func(release.Releaser) bool) ([]release.Releaser,
}
for _, recs := range mem.cache[namespace] {
recs.Iter(func(_ int, rec *record) bool {
if filter(rec.rls) {
ls = append(ls, rec.rls)
// Copy before filtering so the callback cannot reach the
// stored release either.
rls := copyRelease(rec.rls)
if filter(rls) {
ls = append(ls, rls)
}
return true
})
@ -137,7 +140,7 @@ func (mem *Memory) Query(keyvals map[string]string) ([]release.Releaser, error)
return false
}
if rec.lbs.match(lbs) {
ls = append(ls, rec.rls)
ls = append(ls, copyRelease(rec.rls))
}
return true
})

@ -264,3 +264,82 @@ func TestMemoryDelete(t *testing.T) {
}
}
}
// A stored release should only change through Update, which is the contract the
// persistent drivers already provide by encoding on write and decoding on read.
func TestMemoryReleaseIsolation(t *testing.T) {
key := testKey("copy-me", 1)
tests := []struct {
desc string
mutate func(t *testing.T, mem *Memory, rls *rspb.Release)
}{
{
desc: "create input",
mutate: func(_ *testing.T, _ *Memory, rls *rspb.Release) {
rls.Info.Status = common.StatusFailed
},
},
{
desc: "update input",
mutate: func(t *testing.T, mem *Memory, rls *rspb.Release) {
t.Helper()
require.NoError(t, mem.Update(key, rls))
rls.Info.Status = common.StatusFailed
},
},
{
desc: "get result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
rel, err := mem.Get(key)
require.NoError(t, err)
convertReleaserToV1(t, rel).Info.Status = common.StatusFailed
},
},
{
desc: "list result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
ls, err := mem.List(func(release.Releaser) bool { return true })
require.NoError(t, err)
require.Len(t, ls, 1)
convertReleaserToV1(t, ls[0]).Info.Status = common.StatusFailed
},
},
{
desc: "list filter argument",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
_, err := mem.List(func(rel release.Releaser) bool {
convertReleaserToV1(t, rel).Info.Status = common.StatusFailed
return false
})
require.NoError(t, err)
},
},
{
desc: "query result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
ls, err := mem.Query(map[string]string{"name": "copy-me"})
require.NoError(t, err)
require.Len(t, ls, 1)
convertReleaserToV1(t, ls[0]).Info.Status = common.StatusFailed
},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
mem := NewMemory()
rls := releaseFixture()
require.NoError(t, mem.Create(key, rls))
tt.mutate(t, mem, rls)
stored, err := mem.Get(key)
require.NoError(t, err)
assert.Equal(t, common.StatusDeployed, convertReleaserToV1(t, stored).Info.Status)
})
}
}

@ -123,5 +123,7 @@ func newRecord(key string, rls *rspb.Release) *record {
lbs.set("status", rls.Info.Status.String())
lbs.set("version", strconv.Itoa(rls.Version))
return &record{key: key, lbs: lbs, rls: rls}
// Store a copy so later changes to the caller's release do not reach the
// record, matching the isolation the persistent drivers get from encoding.
return &record{key: key, lbs: lbs, rls: copyRelease(rls)}
}

@ -0,0 +1,223 @@
/*
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 driver
import (
"bytes"
"maps"
"slices"
"k8s.io/apimachinery/pkg/runtime"
"helm.sh/helm/v4/pkg/chart/common"
chart "helm.sh/helm/v4/pkg/chart/v2"
rspb "helm.sh/helm/v4/pkg/release/v1"
)
// copyRelease returns a deep copy of rls.
//
// The persistent drivers serialize releases on write and deserialize them on
// read, so callers can never reach the stored data. The memory driver keeps
// releases as Go values, and therefore has to copy them explicitly to offer the
// same isolation.
//
// Values held in the release that Helm does not model, such as custom types
// stored in Config, are copied as interface values rather than recursively.
func copyRelease(rls *rspb.Release) *rspb.Release {
if rls == nil {
return nil
}
return &rspb.Release{
Name: rls.Name,
Info: copyInfo(rls.Info),
Chart: copyChart(rls.Chart),
Config: copyValues(rls.Config),
Manifest: rls.Manifest,
Hooks: copyHooks(rls.Hooks),
Version: rls.Version,
Namespace: rls.Namespace,
Labels: maps.Clone(rls.Labels),
ApplyMethod: rls.ApplyMethod,
}
}
func copyInfo(info *rspb.Info) *rspb.Info {
if info == nil {
return nil
}
out := *info
if info.Resources != nil {
out.Resources = make(map[string][]runtime.Object, len(info.Resources))
for name, objs := range info.Resources {
copied := make([]runtime.Object, len(objs))
for i, obj := range objs {
if obj != nil {
copied[i] = obj.DeepCopyObject()
}
}
out.Resources[name] = copied
}
}
return &out
}
func copyHooks(hooks []*rspb.Hook) []*rspb.Hook {
if hooks == nil {
return nil
}
out := make([]*rspb.Hook, len(hooks))
for i, hook := range hooks {
if hook == nil {
continue
}
copied := *hook
copied.Events = slices.Clone(hook.Events)
copied.DeletePolicies = slices.Clone(hook.DeletePolicies)
copied.OutputLogPolicies = slices.Clone(hook.OutputLogPolicies)
out[i] = &copied
}
return out
}
func copyChart(ch *chart.Chart) *chart.Chart {
if ch == nil {
return nil
}
out := &chart.Chart{
Raw: copyFiles(ch.Raw),
Metadata: copyMetadata(ch.Metadata),
Lock: copyLock(ch.Lock),
Templates: copyFiles(ch.Templates),
Values: copyValues(ch.Values),
Schema: bytes.Clone(ch.Schema),
SchemaModTime: ch.SchemaModTime,
Files: copyFiles(ch.Files),
ModTime: ch.ModTime,
}
// AddDependency sets the parent of each dependency, which rebuilds the
// chart tree without reaching into the unexported fields.
for _, dep := range ch.Dependencies() {
out.AddDependency(copyChart(dep))
}
return out
}
func copyMetadata(md *chart.Metadata) *chart.Metadata {
if md == nil {
return nil
}
out := *md
out.Sources = slices.Clone(md.Sources)
out.Keywords = slices.Clone(md.Keywords)
out.Annotations = maps.Clone(md.Annotations)
if md.Maintainers != nil {
out.Maintainers = make([]*chart.Maintainer, len(md.Maintainers))
for i, m := range md.Maintainers {
if m == nil {
continue
}
copied := *m
out.Maintainers[i] = &copied
}
}
out.Dependencies = copyDependencies(md.Dependencies)
return &out
}
func copyLock(lock *chart.Lock) *chart.Lock {
if lock == nil {
return nil
}
out := *lock
out.Dependencies = copyDependencies(lock.Dependencies)
return &out
}
func copyDependencies(deps []*chart.Dependency) []*chart.Dependency {
if deps == nil {
return nil
}
out := make([]*chart.Dependency, len(deps))
for i, dep := range deps {
if dep == nil {
continue
}
copied := *dep
copied.Tags = slices.Clone(dep.Tags)
copied.ImportValues = copyValueSlice(dep.ImportValues)
out[i] = &copied
}
return out
}
func copyFiles(files []*common.File) []*common.File {
if files == nil {
return nil
}
out := make([]*common.File, len(files))
for i, file := range files {
if file == nil {
continue
}
copied := *file
copied.Data = bytes.Clone(file.Data)
out[i] = &copied
}
return out
}
// copyValues deep copies the maps and slices that make up decoded chart values
// and release config. Other values are copied as interface values, which is
// enough for the scalars these maps normally hold.
func copyValues(values map[string]any) map[string]any {
if values == nil {
return nil
}
out := make(map[string]any, len(values))
for key, value := range values {
out[key] = copyValue(value)
}
return out
}
func copyValue(value any) any {
switch typed := value.(type) {
case map[string]any:
return copyValues(typed)
case map[any]any:
out := make(map[any]any, len(typed))
for key, nested := range typed {
out[key] = copyValue(nested)
}
return out
case []any:
return copyValueSlice(typed)
default:
return value
}
}
func copyValueSlice(values []any) []any {
if values == nil {
return nil
}
out := make([]any, len(values))
for i, value := range values {
out[i] = copyValue(value)
}
return out
}

@ -0,0 +1,114 @@
/*
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 driver
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"helm.sh/helm/v4/pkg/chart/common"
chart "helm.sh/helm/v4/pkg/chart/v2"
rcommon "helm.sh/helm/v4/pkg/release/common"
rspb "helm.sh/helm/v4/pkg/release/v1"
)
// chartFixture builds a chart with a dependency, raw files and nested values so
// the copy can be checked against the parts a JSON round-trip would drop.
func chartFixture() *chart.Chart {
ch := &chart.Chart{
Metadata: &chart.Metadata{
Name: "parent",
Annotations: map[string]string{"example.com/team": "helm"},
Maintainers: []*chart.Maintainer{{Name: "maintainer"}},
},
Raw: []*common.File{{Name: "Chart.yaml", Data: []byte("name: parent")}},
Templates: []*common.File{{Name: "templates/cm.yaml", Data: []byte("kind: ConfigMap")}},
Values: map[string]any{"nested": map[string]any{"replicas": 1}},
Schema: []byte("schema-bytes"),
}
ch.AddDependency(&chart.Chart{Metadata: &chart.Metadata{Name: "child"}})
return ch
}
func releaseFixture() *rspb.Release {
rls := releaseStub("copy-me", 1, "default", rcommon.StatusDeployed)
rls.Chart = chartFixture()
rls.Config = map[string]any{"nested": map[string]any{"enabled": true}}
rls.Hooks = []*rspb.Hook{{
Name: "pre-install",
Events: []rspb.HookEvent{rspb.HookPreInstall},
}}
return rls
}
func TestCopyReleaseIsIndependent(t *testing.T) {
rls := releaseFixture()
copied := copyRelease(rls)
require.NotSame(t, rls, copied)
assert.Equal(t, rls, copied, "a copy should hold the same data as the original")
copied.Info.Status = rcommon.StatusFailed
copied.Labels["key1"] = "changed"
copied.Config["nested"].(map[string]any)["enabled"] = false
copied.Chart.Metadata.Name = "renamed"
copied.Chart.Metadata.Annotations["example.com/team"] = "changed"
copied.Chart.Metadata.Maintainers[0].Name = "changed"
copied.Chart.Values["nested"].(map[string]any)["replicas"] = 2
copied.Chart.Raw[0].Data[0] = 'X'
copied.Chart.Schema[0] = 'X'
copied.Chart.Dependencies()[0].Metadata.Name = "changed"
copied.Hooks[0].Events[0] = rspb.HookPostInstall
assert.Equal(t, rcommon.StatusDeployed, rls.Info.Status)
assert.Equal(t, "val1", rls.Labels["key1"])
assert.Equal(t, true, rls.Config["nested"].(map[string]any)["enabled"])
assert.Equal(t, "parent", rls.Chart.Metadata.Name)
assert.Equal(t, "helm", rls.Chart.Metadata.Annotations["example.com/team"])
assert.Equal(t, "maintainer", rls.Chart.Metadata.Maintainers[0].Name)
assert.Equal(t, 1, rls.Chart.Values["nested"].(map[string]any)["replicas"])
assert.Equal(t, []byte("name: parent"), rls.Chart.Raw[0].Data)
assert.Equal(t, []byte("schema-bytes"), rls.Chart.Schema)
assert.Equal(t, "child", rls.Chart.Dependencies()[0].Metadata.Name)
assert.Equal(t, rspb.HookPreInstall, rls.Hooks[0].Events[0])
}
// The chart tree is rebuilt through AddDependency, so the copied dependency has
// to point at the copied parent rather than the original one.
func TestCopyReleaseRebuildsChartTree(t *testing.T) {
copied := copyRelease(releaseFixture())
require.Len(t, copied.Chart.Dependencies(), 1)
dep := copied.Chart.Dependencies()[0]
assert.Same(t, copied.Chart, dep.Parent())
assert.True(t, copied.Chart.IsRoot())
assert.Equal(t, "parent.child", dep.ChartPath())
}
func TestCopyReleaseKeepsNilFields(t *testing.T) {
copied := copyRelease(&rspb.Release{Name: "sparse"})
assert.Equal(t, "sparse", copied.Name)
assert.Nil(t, copied.Info)
assert.Nil(t, copied.Chart)
assert.Nil(t, copied.Config)
assert.Nil(t, copied.Hooks)
assert.Nil(t, copied.Labels)
assert.Nil(t, copyRelease(nil))
}

@ -79,7 +79,7 @@ func (mem *Memory) Get(key string) (release.Releaser, error) {
}
if recs, ok := mem.cache[mem.namespace][name]; ok {
if r := recs.Get(key); r != nil {
return r.rls, nil
return copyRelease(r.rls), nil
}
}
return nil, ErrReleaseNotFound
@ -100,8 +100,11 @@ func (mem *Memory) List(filter func(release.Releaser) bool) ([]release.Releaser,
}
for _, recs := range mem.cache[namespace] {
recs.Iter(func(_ int, rec *record) bool {
if filter(rec.rls) {
ls = append(ls, rec.rls)
// Copy before filtering so the callback cannot reach the
// stored release either.
rls := copyRelease(rec.rls)
if filter(rls) {
ls = append(ls, rls)
}
return true
})
@ -137,7 +140,7 @@ func (mem *Memory) Query(keyvals map[string]string) ([]release.Releaser, error)
return false
}
if rec.lbs.match(lbs) {
ls = append(ls, rec.rls)
ls = append(ls, copyRelease(rec.rls))
}
return true
})

@ -264,3 +264,82 @@ func TestMemoryDelete(t *testing.T) {
}
}
}
// A stored release should only change through Update, which is the contract the
// persistent drivers already provide by encoding on write and decoding on read.
func TestMemoryReleaseIsolation(t *testing.T) {
key := testKey("copy-me", 1)
tests := []struct {
desc string
mutate func(t *testing.T, mem *Memory, rls *rspb.Release)
}{
{
desc: "create input",
mutate: func(_ *testing.T, _ *Memory, rls *rspb.Release) {
rls.Info.Status = common.StatusFailed
},
},
{
desc: "update input",
mutate: func(t *testing.T, mem *Memory, rls *rspb.Release) {
t.Helper()
require.NoError(t, mem.Update(key, rls))
rls.Info.Status = common.StatusFailed
},
},
{
desc: "get result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
rel, err := mem.Get(key)
require.NoError(t, err)
convertReleaserToV1(t, rel).Info.Status = common.StatusFailed
},
},
{
desc: "list result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
ls, err := mem.List(func(release.Releaser) bool { return true })
require.NoError(t, err)
require.Len(t, ls, 1)
convertReleaserToV1(t, ls[0]).Info.Status = common.StatusFailed
},
},
{
desc: "list filter argument",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
_, err := mem.List(func(rel release.Releaser) bool {
convertReleaserToV1(t, rel).Info.Status = common.StatusFailed
return false
})
require.NoError(t, err)
},
},
{
desc: "query result",
mutate: func(t *testing.T, mem *Memory, _ *rspb.Release) {
t.Helper()
ls, err := mem.Query(map[string]string{"name": "copy-me"})
require.NoError(t, err)
require.Len(t, ls, 1)
convertReleaserToV1(t, ls[0]).Info.Status = common.StatusFailed
},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
mem := NewMemory()
rls := releaseFixture()
require.NoError(t, mem.Create(key, rls))
tt.mutate(t, mem, rls)
stored, err := mem.Get(key)
require.NoError(t, err)
assert.Equal(t, common.StatusDeployed, convertReleaserToV1(t, stored).Info.Status)
})
}
}

@ -123,6 +123,7 @@ func newRecord(key string, rls *rspb.Release) *record {
lbs.set("status", rls.Info.Status.String())
lbs.set("version", strconv.Itoa(rls.Version))
// return &record{key: key, lbs: lbs, rls: proto.Clone(rls).(*rspb.Release)}
return &record{key: key, lbs: lbs, rls: rls}
// Store a copy so later changes to the caller's release do not reach the
// record, matching the isolation the persistent drivers get from encoding.
return &record{key: key, lbs: lbs, rls: copyRelease(rls)}
}

Loading…
Cancel
Save