Skip to content

Commit

Permalink
Merge pull request #697 from fluxcd/ssa-break-cycle
Browse files Browse the repository at this point in the history
ssa: avoid potential cyclic import
  • Loading branch information
hiddeco authored Dec 1, 2023
2 parents 1ad0559 + f3cc2d7 commit 6637feb
Show file tree
Hide file tree
Showing 26 changed files with 744 additions and 554 deletions.
10 changes: 6 additions & 4 deletions ssa/errors.go → ssa/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package ssa
package errors

import (
"fmt"

apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

"github.com/fluxcd/pkg/ssa/utils"
)

// DryRunErr is an error that occurs during a server-side dry-run apply.
Expand Down Expand Up @@ -51,9 +53,9 @@ func (e *DryRunErr) Error() string {

if apierrors.IsNotFound(e.Unwrap()) {
if e.involvedObject.GetNamespace() != "" {
return fmt.Sprintf("%s namespace not specified: %s", FmtUnstructured(e.involvedObject), e.Unwrap().Error())
return fmt.Sprintf("%s namespace not specified: %s", utils.FmtUnstructured(e.involvedObject), e.Unwrap().Error())
}
return fmt.Sprintf("%s not found: %s", FmtUnstructured(e.involvedObject), e.Unwrap().Error())
return fmt.Sprintf("%s not found: %s", utils.FmtUnstructured(e.involvedObject), e.Unwrap().Error())
}

reason := string(apierrors.ReasonForError(e.Unwrap()))
Expand All @@ -67,7 +69,7 @@ func (e *DryRunErr) Error() string {
reason = fmt.Sprintf(" (%s)", reason)
}

return fmt.Sprintf("%s dry-run failed%s: %s", FmtUnstructured(e.involvedObject), reason, e.underlyingErr.Error())
return fmt.Sprintf("%s dry-run failed%s: %s", utils.FmtUnstructured(e.involvedObject), reason, e.underlyingErr.Error())
}

// Unwrap returns the underlying error.
Expand Down
48 changes: 48 additions & 0 deletions ssa/errors/immutable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Copyright 2023 The Flux 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 errors

import (
"regexp"

"k8s.io/apimachinery/pkg/api/errors"
)

// Match CEL immutable error variants.
var matchImmutableFieldErrors = []*regexp.Regexp{
regexp.MustCompile(`.*is\simmutable.*`),
regexp.MustCompile(`.*immutable\sfield.*`),
}

// IsImmutableError checks if the given error is an immutable error.
func IsImmutableError(err error) bool {
// Detect immutability like kubectl does
// https://github.com/kubernetes/kubectl/blob/8165f83007/pkg/cmd/apply/patcher.go#L201
if errors.IsConflict(err) || errors.IsInvalid(err) {
return true
}

// Detect immutable errors returned by custom admission webhooks and Kubernetes CEL
// https://kubernetes.io/blog/2022/09/29/enforce-immutability-using-cel/#immutablility-after-first-modification
for _, fieldError := range matchImmutableFieldErrors {
if fieldError.MatchString(err.Error()) {
return true
}
}

return false
}
56 changes: 56 additions & 0 deletions ssa/errors/immutable_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
Copyright 2023 The Flux 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 errors

import (
"fmt"
"testing"

. "github.com/onsi/gomega"
)

func TestIsImmutableError(t *testing.T) {
testCases := []struct {
name string
err error
match bool
}{
{
name: "CEL immutable error",
err: fmt.Errorf(`the ImmutableSinceFirstWrite "test1" is invalid: value: Invalid value: "string": Value is immutable`),
match: true,
},
{
name: "Custom admission immutable error",
err: fmt.Errorf(`the IAMPolicyMember's spec is immutable: admission webhook "deny-immutable-field-updates.cnrm.cloud.google.com" denied the request: the IAMPolicyMember's spec is immutable`),
match: true,
},
{
name: "Not immutable error",
err: fmt.Errorf(`is not immutable`),
match: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

g.Expect(IsImmutableError(tc.err)).To(BeIdenticalTo(tc.match))
})
}
}
4 changes: 2 additions & 2 deletions ssa/jsondiff/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/envtest"

"github.com/fluxcd/pkg/ssa"
"github.com/fluxcd/pkg/ssa/utils"
)

var (
Expand Down Expand Up @@ -80,5 +80,5 @@ func LoadResource(p string) (*unstructured.Unstructured, error) {
return nil, err
}
defer f.Close()
return ssa.ReadObject(f)
return utils.ReadObject(f)
}
10 changes: 6 additions & 4 deletions ssa/jsondiff/unstructured.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import (
"k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/fluxcd/pkg/ssa"
ssaerrors "github.com/fluxcd/pkg/ssa/errors"
"github.com/fluxcd/pkg/ssa/normalize"
"github.com/fluxcd/pkg/ssa/utils"
)

// IgnorePathRoot ignores the root of a JSON document, i.e., the entire
Expand Down Expand Up @@ -118,7 +120,7 @@ func Unstructured(ctx context.Context, c client.Client, obj *unstructured.Unstru
o.ApplyOptions(opts)

// Check if the object should be excluded based on metadata.
if ssa.AnyInMetadata(obj, o.ExclusionSelector) {
if utils.AnyInMetadata(obj, o.ExclusionSelector) {
return NewDiffForUnstructured(obj, nil, DiffTypeExclude, nil), nil
}

Expand All @@ -142,14 +144,14 @@ func Unstructured(ctx context.Context, c client.Client, obj *unstructured.Unstru
client.FieldOwner(o.FieldManager),
}
if err := c.Patch(ctx, dryRunObj, client.Apply, patchOpts...); err != nil {
return nil, ssa.NewDryRunErr(err, obj)
return nil, ssaerrors.NewDryRunErr(err, obj)
}

if dryRunObj.GetResourceVersion() == "" {
return NewDiffForUnstructured(obj, nil, DiffTypeCreate, nil), nil
}

if err := ssa.NormalizeDryRunUnstructured(dryRunObj); err != nil {
if err := normalize.DryRunUnstructured(dryRunObj); err != nil {
return nil, err
}

Expand Down
18 changes: 9 additions & 9 deletions ssa/jsondiff/unstructured_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/fluxcd/pkg/ssa"
"github.com/fluxcd/pkg/ssa/normalize"
)

const dummyFieldOwner = "dummy"
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestUnstructuredList(t *testing.T) {
},
mutateDesired: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "foo")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
opts: []ListOption{
MaskSecrets(true),
Expand Down Expand Up @@ -304,14 +304,14 @@ func TestUnstructuredList(t *testing.T) {
"a": "2",
"b": "1",
}, "data")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
mutateDesired: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedMap(obj.Object, map[string]interface{}{
"a": "1",
"b": "2",
}, "data")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
opts: []ListOption{
Rationalize(true),
Expand Down Expand Up @@ -718,7 +718,7 @@ func TestUnstructured(t *testing.T) {
path: "testdata/empty-secret.yaml",
mutateDesired: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "foo")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
opts: []ResourceOption{
MaskSecrets(false),
Expand All @@ -742,11 +742,11 @@ func TestUnstructured(t *testing.T) {
mutateCluster: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "foo")
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "bar")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
mutateDesired: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "baz", "stringData", "foo")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
opts: []ResourceOption{
MaskSecrets(true),
Expand All @@ -769,11 +769,11 @@ func TestUnstructured(t *testing.T) {
mutateCluster: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "foo")
_ = unstructured.SetNestedField(obj.Object, "bar", "stringData", "bar")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
mutateDesired: func(obj *unstructured.Unstructured) {
_ = unstructured.SetNestedField(obj.Object, "baz", "stringData", "foo")
_ = ssa.NormalizeUnstructured(obj)
_ = normalize.Unstructured(obj)
},
opts: []ResourceOption{
MaskSecrets(true),
Expand Down
6 changes: 4 additions & 2 deletions ssa/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/envtest"

"github.com/fluxcd/cli-utils/pkg/kstatus/polling"

"github.com/fluxcd/pkg/ssa/utils"
)

var manager *ResourceManager
Expand Down Expand Up @@ -85,7 +87,7 @@ func readManifest(manifest, namespace string) ([]*unstructured.Unstructured, err
}
yml := fmt.Sprintf(string(data), namespace)

objects, err := ReadObjects(strings.NewReader(yml))
objects, err := utils.ReadObjects(strings.NewReader(yml))
if err != nil {
return nil, err
}
Expand All @@ -103,7 +105,7 @@ func generateName(prefix string) string {
func getFirstObject(objects []*unstructured.Unstructured, kind, name string) (string, *unstructured.Unstructured) {
for _, object := range objects {
if object.GetKind() == kind && object.GetName() == name {
return FmtUnstructured(object), object
return utils.FmtUnstructured(object), object
}
}
return "", nil
Expand Down
4 changes: 3 additions & 1 deletion ssa/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (

"github.com/fluxcd/cli-utils/pkg/kstatus/polling"
"github.com/fluxcd/cli-utils/pkg/object"

"github.com/fluxcd/pkg/ssa/utils"
)

// ResourceManager reconciles Kubernetes resources onto the target cluster using server-side apply.
Expand Down Expand Up @@ -87,7 +89,7 @@ func (m *ResourceManager) changeSetEntry(o *unstructured.Unstructured, action Ac
return &ChangeSetEntry{
ObjMetadata: object.UnstructuredToObjMetadata(o),
GroupVersion: o.GroupVersionKind().Version,
Subject: FmtUnstructured(o),
Subject: utils.FmtUnstructured(o),
Action: action,
}
}
Loading

0 comments on commit 6637feb

Please sign in to comment.