Skip to content

Commit

Permalink
ssa: avoid potential cyclic import
Browse files Browse the repository at this point in the history
This moves things into separate packages to avoid a potential cyclic
import as soon as we would like to utilize `jsondiff` in `ssa` itself.

Signed-off-by: Hidde Beydals <hidde@hhh.computer>
  • Loading branch information
hiddeco committed Dec 1, 2023
1 parent 1ad0559 commit e7f4397
Show file tree
Hide file tree
Showing 26 changed files with 712 additions and 539 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.Unstructured(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.Unstructured(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.Unstructured(e.involvedObject), reason, e.underlyingErr.Error())
}

// Unwrap returns the underlying error.
Expand Down
32 changes: 32 additions & 0 deletions ssa/errors/immutable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
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
}
40 changes: 40 additions & 0 deletions ssa/errors/immutable_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
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.Unstructured(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.Unstructured(o),
Action: action,
}
}
Loading

0 comments on commit e7f4397

Please sign in to comment.