Skip to content

Commit a7fe438

Browse files
authored
Merge pull request #52 from banzaicloud/patchmaker-interfaces
make patch maker dependency implementations easily changeable
2 parents 7e08045 + 6ba17df commit a7fe438

File tree

9 files changed

+148
-45
lines changed

9 files changed

+148
-45
lines changed

.circleci/config.yml

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,35 +145,44 @@ jobs:
145145
name: Run verification
146146
command: make
147147

148+
integration-test-k8s1-23:
149+
<<: *integration-test-base
150+
environment:
151+
<<: *integration-test-environment
152+
K8S_VERSION: v1.23.4
153+
148154
integration-test-k8s1-22:
149155
<<: *integration-test-base
150156
environment:
151157
<<: *integration-test-environment
152-
K8S_VERSION: v1.22.3
158+
K8S_VERSION: v1.22.7
153159

154160
integration-test-k8s1-21:
155161
<<: *integration-test-base
156162
environment:
157163
<<: *integration-test-environment
158-
K8S_VERSION: v1.21.3
164+
K8S_VERSION: v1.21.10
159165

160166
integration-test-k8s1-20:
161167
<<: *integration-test-base
162168
environment:
163169
<<: *integration-test-environment
164-
K8S_VERSION: v1.20.9
170+
K8S_VERSION: v1.20.15
165171

166172
integration-test-k8s1-19:
167173
<<: *integration-test-base
168174
environment:
169175
<<: *integration-test-environment
170-
K8S_VERSION: v1.19.2
176+
K8S_VERSION: v1.19.16
171177

172178
workflows:
173179
version: 2
174180
ci:
175181
jobs:
176182
- check
183+
- integration-test-k8s1-23:
184+
requires:
185+
- check
177186
- integration-test-k8s1-22:
178187
requires:
179188
- check

.github/mergeable.yml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
version: 2
2+
mergeable:
3+
- when: pull_request.*
4+
validate:
5+
- do: title
6+
must_exclude:
7+
regex: '^(\[wip\]|wip:)'
8+
message: 'WIP tag in PR title'
9+
- do: label
10+
must_exclude:
11+
regex: 'wip'
12+
message: 'WIP label on PR'
13+
- do: description
14+
and:
15+
- must_exclude:
16+
regex: '\[ \]'
17+
message: 'Remaining tasks in the description.'
18+
- must_exclude:
19+
regex: 'no\|yes|fixes #X, partially #Y, mentioned in #Z'
20+
message: 'Please fill out the PR template.'

.golangci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ linters:
1515
enable:
1616
- misspell
1717
- gofmt
18-
- golint
1918
- goimports
2019
disable:
20+
- golint
2121
- errcheck
2222
- gas
2323
- megacheck

Makefile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
LICENSEI_VERSION = 0.4.0
2-
GOLANGCI_VERSION = 1.16.0
1+
LICENSEI_VERSION = 0.5.0
2+
GOLANGCI_VERSION = 1.44.0
33

44
all: license fmt vet test
55

@@ -42,7 +42,7 @@ bin/golangci-lint: bin/golangci-lint-${GOLANGCI_VERSION}
4242
@ln -sf golangci-lint-${GOLANGCI_VERSION} bin/golangci-lint
4343
bin/golangci-lint-${GOLANGCI_VERSION}:
4444
@mkdir -p bin
45-
curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | bash -s -- -b ./bin/ v${GOLANGCI_VERSION}
45+
curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/v${GOLANGCI_VERSION}/install.sh | bash -s -- -b ./bin/ v${GOLANGCI_VERSION}
4646
@mv bin/golangci-lint $@
4747

4848
.PHONY: lint

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ go 1.15
55
require (
66
emperror.dev/errors v0.8.0
77
github.com/evanphx/json-patch v4.9.0+incompatible
8-
github.com/json-iterator/go v1.1.11
8+
github.com/json-iterator/go v1.1.12
99
k8s.io/apimachinery v0.19.2
1010
)

go.sum

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
6161
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
6262
github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
6363
github.com/json-iterator/go v1.1.10/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
64-
github.com/json-iterator/go v1.1.11 h1:uVUAXhF2To8cbw/3xN3pxj6kk7TYKs98NIrTqPlMWAQ=
65-
github.com/json-iterator/go v1.1.11/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
64+
github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM=
65+
github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
6666
github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
6767
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
6868
github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs=
@@ -75,8 +75,9 @@ github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJ
7575
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg=
7676
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
7777
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
78-
github.com/modern-go/reflect2 v1.0.1 h1:9f412s+6RmYXLWZSEzVVgPGK7C2PphHj5RJrvfx9AWI=
7978
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
79+
github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M=
80+
github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk=
8081
github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
8182
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
8283
github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=

patch/patch.go

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,30 @@ import (
1818
"fmt"
1919

2020
"emperror.dev/errors"
21-
jsonpatch "github.com/evanphx/json-patch"
2221
json "github.com/json-iterator/go"
2322
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2423
"k8s.io/apimachinery/pkg/runtime"
25-
"k8s.io/apimachinery/pkg/util/jsonmergepatch"
26-
"k8s.io/apimachinery/pkg/util/strategicpatch"
2724
)
2825

29-
var DefaultPatchMaker = NewPatchMaker(DefaultAnnotator)
26+
var DefaultPatchMaker = NewPatchMaker(DefaultAnnotator, &K8sStrategicMergePatcher{}, &BaseJSONMergePatcher{})
27+
28+
type Maker interface {
29+
Calculate(currentObject, modifiedObject runtime.Object, opts ...CalculateOption) (*PatchResult, error)
30+
}
3031

3132
type PatchMaker struct {
3233
annotator *Annotator
34+
35+
strategicMergePatcher StrategicMergePatcher
36+
jsonMergePatcher JSONMergePatcher
3337
}
3438

35-
func NewPatchMaker(annotator *Annotator) *PatchMaker {
39+
func NewPatchMaker(annotator *Annotator, strategicMergePatcher StrategicMergePatcher, jsonMergePatcher JSONMergePatcher) Maker {
3640
return &PatchMaker{
3741
annotator: annotator,
42+
43+
strategicMergePatcher: strategicMergePatcher,
44+
jsonMergePatcher: jsonMergePatcher,
3845
}
3946
}
4047

@@ -75,28 +82,24 @@ func (p *PatchMaker) Calculate(currentObject, modifiedObject runtime.Object, opt
7582

7683
switch currentObject.(type) {
7784
default:
78-
lookupPatchMeta, err := strategicpatch.NewPatchMetaFromStruct(currentObject)
79-
if err != nil {
80-
return nil, errors.WrapWithDetails(err, "Failed to lookup patch meta", "current object", currentObject)
81-
}
82-
patch, err = strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, true)
85+
patch, err = p.strategicMergePatcher.CreateThreeWayMergePatch(original, modified, current, currentObject)
8386
if err != nil {
8487
return nil, errors.Wrap(err, "Failed to generate strategic merge patch")
8588
}
8689
// $setElementOrder can make it hard to decide whether there is an actual diff or not.
8790
// In cases like that trying to apply the patch locally on current will make it clear.
8891
if string(patch) != "{}" {
89-
patchCurrent, err := strategicpatch.StrategicMergePatch(current, patch, currentObject)
92+
patchCurrent, err := p.strategicMergePatcher.StrategicMergePatch(current, patch, currentObject)
9093
if err != nil {
9194
return nil, errors.Wrap(err, "Failed to apply patch again to check for an actual diff")
9295
}
93-
patch, err = strategicpatch.CreateTwoWayMergePatch(current, patchCurrent, currentObject)
96+
patch, err = p.strategicMergePatcher.CreateTwoWayMergePatch(current, patchCurrent, currentObject)
9497
if err != nil {
9598
return nil, errors.Wrap(err, "Failed to create patch again to check for an actual diff")
9699
}
97100
}
98101
case *unstructured.Unstructured:
99-
patch, err = unstructuredJsonMergePatch(original, modified, current)
102+
patch, err = p.unstructuredJsonMergePatch(original, modified, current)
100103
if err != nil {
101104
return nil, errors.Wrap(err, "Failed to generate merge patch")
102105
}
@@ -110,38 +113,38 @@ func (p *PatchMaker) Calculate(currentObject, modifiedObject runtime.Object, opt
110113
}, nil
111114
}
112115

113-
type PatchResult struct {
114-
Patch []byte
115-
Current []byte
116-
Modified []byte
117-
Original []byte
118-
}
119-
120-
func (p *PatchResult) IsEmpty() bool {
121-
return string(p.Patch) == "{}"
122-
}
123-
124-
func (p *PatchResult) String() string {
125-
return fmt.Sprintf("\nPatch: %s \nCurrent: %s\nModified: %s\nOriginal: %s\n", p.Patch, p.Current, p.Modified, p.Original)
126-
}
127-
128-
func unstructuredJsonMergePatch(original, modified, current []byte) ([]byte, error) {
129-
patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current)
116+
func (p *PatchMaker) unstructuredJsonMergePatch(original, modified, current []byte) ([]byte, error) {
117+
patch, err := p.jsonMergePatcher.CreateThreeWayJSONMergePatch(original, modified, current)
130118
if err != nil {
131119
return nil, errors.Wrap(err, "Failed to generate merge patch")
132120
}
133121
// Apply the patch to the current object and create a merge patch to see if there has any effective changes been made
134122
if string(patch) != "{}" {
135123
// apply the patch
136-
patchedCurrent, err := jsonpatch.MergePatch(current, patch)
124+
patchedCurrent, err := p.jsonMergePatcher.MergePatch(current, patch)
137125
if err != nil {
138126
return nil, errors.Wrap(err, "Failed to merge generated patch to current object")
139127
}
140128
// create the patch again, but now between the current and the patched version of the current object
141-
patch, err = jsonpatch.CreateMergePatch(current, patchedCurrent)
129+
patch, err = p.jsonMergePatcher.CreateMergePatch(current, patchedCurrent)
142130
if err != nil {
143131
return nil, errors.Wrap(err, "Failed to create patch between the current and patched current object")
144132
}
145133
}
146134
return patch, err
147135
}
136+
137+
type PatchResult struct {
138+
Patch []byte
139+
Current []byte
140+
Modified []byte
141+
Original []byte
142+
}
143+
144+
func (p *PatchResult) IsEmpty() bool {
145+
return string(p.Patch) == "{}"
146+
}
147+
148+
func (p *PatchResult) String() string {
149+
return fmt.Sprintf("\nPatch: %s \nCurrent: %s\nModified: %s\nOriginal: %s\n", p.Patch, p.Current, p.Modified, p.Original)
150+
}

patch/patch_makers.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright © 2022 Banzai Cloud
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package patch
16+
17+
import (
18+
"emperror.dev/errors"
19+
jsonpatch "github.com/evanphx/json-patch"
20+
"k8s.io/apimachinery/pkg/util/jsonmergepatch"
21+
"k8s.io/apimachinery/pkg/util/mergepatch"
22+
"k8s.io/apimachinery/pkg/util/strategicpatch"
23+
)
24+
25+
type StrategicMergePatcher interface {
26+
StrategicMergePatch(original, patch []byte, dataStruct interface{}) ([]byte, error)
27+
CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}) ([]byte, error)
28+
CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}) ([]byte, error)
29+
}
30+
31+
type JSONMergePatcher interface {
32+
MergePatch(docData, patchData []byte) ([]byte, error)
33+
CreateMergePatch(originalJSON, modifiedJSON []byte) ([]byte, error)
34+
CreateThreeWayJSONMergePatch(original, modified, current []byte) ([]byte, error)
35+
}
36+
37+
type K8sStrategicMergePatcher struct {
38+
PreconditionFuncs []mergepatch.PreconditionFunc
39+
}
40+
41+
func (p *K8sStrategicMergePatcher) StrategicMergePatch(original, patch []byte, dataStruct interface{}) ([]byte, error) {
42+
return strategicpatch.StrategicMergePatch(original, patch, dataStruct)
43+
}
44+
45+
func (p *K8sStrategicMergePatcher) CreateTwoWayMergePatch(original, modified []byte, dataStruct interface{}) ([]byte, error) {
46+
return strategicpatch.CreateTwoWayMergePatch(original, modified, dataStruct, p.PreconditionFuncs...)
47+
}
48+
49+
func (p *K8sStrategicMergePatcher) CreateThreeWayMergePatch(original, modified, current []byte, dataStruct interface{}) ([]byte, error) {
50+
lookupPatchMeta, err := strategicpatch.NewPatchMetaFromStruct(dataStruct)
51+
if err != nil {
52+
return nil, errors.WrapWithDetails(err, "Failed to lookup patch meta", "current object", dataStruct)
53+
}
54+
55+
return strategicpatch.CreateThreeWayMergePatch(original, modified, current, lookupPatchMeta, true, p.PreconditionFuncs...)
56+
}
57+
58+
type BaseJSONMergePatcher struct{}
59+
60+
func (p *BaseJSONMergePatcher) MergePatch(docData, patchData []byte) ([]byte, error) {
61+
return jsonpatch.MergePatch(docData, patchData)
62+
}
63+
64+
func (p *BaseJSONMergePatcher) CreateMergePatch(originalJSON, modifiedJSON []byte) ([]byte, error) {
65+
return jsonpatch.CreateMergePatch(originalJSON, modifiedJSON)
66+
}
67+
68+
func (p *BaseJSONMergePatcher) CreateThreeWayJSONMergePatch(original, modified, current []byte) ([]byte, error) {
69+
return jsonmergepatch.CreateThreeWayJSONMergePatch(original, modified, current)
70+
}

patch/patch_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func Test_unstructuredJsonMergePatch(t *testing.T) {
9292
}
9393
for _, tt := range tests {
9494
t.Run(tt.name, func(t *testing.T) {
95-
got, err := unstructuredJsonMergePatch(
95+
got, err := DefaultPatchMaker.(*PatchMaker).unstructuredJsonMergePatch(
9696
mustFromUnstructured(tt.args.original),
9797
mustFromUnstructured(tt.args.modified),
9898
mustFromUnstructured(tt.args.current))

0 commit comments

Comments
 (0)