Skip to content

Commit 4760bfb

Browse files
KirdockKirdock
authored
refactor(automation): Use Create and Update instead of Upsert (#1962)
#### **Why** this PR? As upsert is very niche and removed from core, the upsert logic is added to Monaco #### **What** has changed? Upsert of core is not used anymore and upsert logic is added to Monaco. #### **How** does it do it? #### How is it **tested**? Existing tests cover it, and I added more tests. #### How does it affect **users**? None **Issue:** CA-15100
1 parent 4c914d3 commit 4760bfb

File tree

6 files changed

+107
-28
lines changed

6 files changed

+107
-28
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.24.3
44

55
require (
66
github.com/anknown/ahocorasick v0.0.0-20190904063843-d75dbd5169c0
7-
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250617130523-bb91a9a27976
7+
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250630105247-bd703ac248dc
88
github.com/go-logr/logr v1.4.3
99
github.com/google/go-cmp v0.7.0
1010
github.com/google/uuid v1.6.0

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx2
99
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
1010
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
1111
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
12-
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250617130523-bb91a9a27976 h1:g4NO6M5pdaLa0Yui9yN5LXIWe2kfNsmSH2WI9kaho34=
13-
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250617130523-bb91a9a27976/go.mod h1:XskCEkvBoQCq3TFYFrQ3KBwyKAyUQGTYUjGKtVaxISY=
12+
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250630105247-bd703ac248dc h1:fO+nKvO6+0BCnM3queAIAU5M1gM6lJiZe/cbMz5RHuY=
13+
github.com/dynatrace/dynatrace-configuration-as-code-core v0.9.1-0.20250630105247-bd703ac248dc/go.mod h1:XskCEkvBoQCq3TFYFrQ3KBwyKAyUQGTYUjGKtVaxISY=
1414
github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI=
1515
github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
1616
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=

‎pkg/client/clientset.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,6 @@ type AutomationClient interface {
154154
Create(ctx context.Context, resourceType automation.ResourceType, data []byte) (result libAPI.Response, err error)
155155
Update(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (libAPI.Response, error)
156156
List(ctx context.Context, resourceType automation.ResourceType) (libAPI.PagedListResponse, error)
157-
Upsert(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (result libAPI.Response, err error)
158157
Delete(ctx context.Context, resourceType automation.ResourceType, id string) (libAPI.Response, error)
159158
}
160159

‎pkg/client/dummy_clientset.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,6 @@ func (d *DummyAutomationClient) List(ctx context.Context, resourceType automatio
6666

6767
// Update implements AutomationClient.
6868
func (d *DummyAutomationClient) Update(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (api.Response, error) {
69-
panic("unimplemented")
70-
}
71-
72-
// Upsert implements AutomationClient.
73-
func (d *DummyAutomationClient) Upsert(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (result api.Response, err error) {
7469
return api.Response{
7570
StatusCode: 200,
7671
Data: []byte(fmt.Sprintf(`{"id" : "%s"}`, id)),

‎pkg/resource/automation/deploy.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package automation
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"fmt"
2223
"time"
2324

@@ -33,7 +34,8 @@ import (
3334

3435
//go:generate mockgen -source=deploy.go -destination=automation_mock.go -package=automation DeploySource
3536
type DeploySource interface {
36-
Upsert(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (result api.Response, err error)
37+
Create(ctx context.Context, resourceType automation.ResourceType, data []byte) (api.Response, error)
38+
Update(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (api.Response, error)
3739
}
3840

3941
type DeployAPI struct {
@@ -66,7 +68,7 @@ func (d DeployAPI) Deploy(ctx context.Context, properties parameter.Properties,
6668
return entities.ResolvedEntity{}, errors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert automation object of type %s with id %s", t.Resource, id)).WithError(err)
6769
}
6870

69-
resp, err := d.source.Upsert(ctx, resourceType, id, []byte(renderedConfig))
71+
resp, err := d.upsert(ctx, resourceType, id, []byte(renderedConfig))
7072
if err != nil {
7173
return entities.ResolvedEntity{}, errors.NewConfigDeployErr(c, fmt.Sprintf("failed to upsert automation object of type %s with id %s", t.Resource, id)).WithError(err)
7274
}
@@ -85,3 +87,38 @@ func (d DeployAPI) Deploy(ctx context.Context, properties parameter.Properties,
8587
return resolved, nil
8688

8789
}
90+
91+
func (d DeployAPI) upsert(ctx context.Context, resourceType automation.ResourceType, id string, data []byte) (api.Response, error) {
92+
resp, err := d.source.Update(ctx, resourceType, id, data)
93+
94+
// return response if there is no error
95+
if err == nil {
96+
return resp, nil
97+
}
98+
99+
// NotFound would mean that we need to create it, if not, something else is happening
100+
if !api.IsNotFoundError(err) {
101+
return api.Response{}, err
102+
}
103+
104+
// make sure actual "id" field is set in payload
105+
if err := setIDField(id, &data); err != nil {
106+
return api.Response{}, fmt.Errorf("failed to create automation resource of type %v with id %s: unable to set the id field in order to create object: %w", resourceType, id, err)
107+
}
108+
109+
return d.source.Create(ctx, resourceType, data)
110+
}
111+
112+
func setIDField(id string, data *[]byte) error {
113+
var m map[string]interface{}
114+
err := json.Unmarshal(*data, &m)
115+
if err != nil {
116+
return err
117+
}
118+
m["id"] = id
119+
*data, err = json.Marshal(m)
120+
if err != nil {
121+
return err
122+
}
123+
return nil
124+
}

‎pkg/resource/automation/deploy_test.go

Lines changed: 65 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ import (
3535
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/resource/automation"
3636
)
3737

38+
var idResponseData = []byte(`{ "id": "config-id" }`)
39+
3840
func TestDeployAutomation_WrongType(t *testing.T) {
3941
client := &client.DummyAutomationClient{}
4042

@@ -60,10 +62,10 @@ func TestDeployAutomation_UnknownResourceType(t *testing.T) {
6062
assert.NotEmpty(t, errs)
6163
}
6264

63-
func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
64-
t.Run("TestDeployAutomation - Workflow Upsert fails", func(t *testing.T) {
65+
func TestDeployAutomation_ClientUpdateFails(t *testing.T) {
66+
t.Run("TestDeployAutomation - Workflow Update fails", func(t *testing.T) {
6567
client := automation.NewMockDeploySource(gomock.NewController(t))
66-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPSERT_FAIL"))
68+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPDATE_FAIL"))
6769

6870
conf := &config.Config{
6971
Type: config.AutomationType{
@@ -76,9 +78,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
7678
assert.Zero(t, resp)
7779
assert.Error(t, err)
7880
})
79-
t.Run("TestDeployAutomation - Workflow Upsert fails - HTTP Err", func(t *testing.T) {
81+
t.Run("TestDeployAutomation - Workflow Update fails - HTTP Err", func(t *testing.T) {
8082
client := automation.NewMockDeploySource(gomock.NewController(t))
81-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{StatusCode: 400}, nil)
83+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, api.APIError{StatusCode: 400})
8284

8385
conf := &config.Config{
8486
Type: config.AutomationType{
@@ -90,9 +92,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
9092
_, err := automation.NewDeployAPI(client).Deploy(t.Context(), nil, "", conf)
9193
assert.Error(t, err)
9294
})
93-
t.Run("TestDeployAutomation - BusinessCalendar Upsert fails", func(t *testing.T) {
95+
t.Run("TestDeployAutomation - BusinessCalendar Update fails", func(t *testing.T) {
9496
client := automation.NewMockDeploySource(gomock.NewController(t))
95-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPSERT_FAIL"))
97+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPDATE_FAIL"))
9698

9799
conf := &config.Config{
98100
Type: config.AutomationType{
@@ -104,9 +106,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
104106
_, err := automation.NewDeployAPI(client).Deploy(t.Context(), nil, "", conf)
105107
assert.Error(t, err)
106108
})
107-
t.Run("TestDeployAutomation - BusinessCalendar Upsert fails - HTTP Error", func(t *testing.T) {
109+
t.Run("TestDeployAutomation - BusinessCalendar Update fails - HTTP Error", func(t *testing.T) {
108110
client := automation.NewMockDeploySource(gomock.NewController(t))
109-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{StatusCode: 400}, nil)
111+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, api.APIError{StatusCode: 400})
110112

111113
conf := &config.Config{
112114
Type: config.AutomationType{
@@ -118,9 +120,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
118120
_, err := automation.NewDeployAPI(client).Deploy(t.Context(), nil, "", conf)
119121
assert.Error(t, err)
120122
})
121-
t.Run("TestDeployAutomation - Scheduling Rule Upsert fails", func(t *testing.T) {
123+
t.Run("TestDeployAutomation - Scheduling Rule Update fails", func(t *testing.T) {
122124
client := automation.NewMockDeploySource(gomock.NewController(t))
123-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPSERT_FAIL"))
125+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, errors.New("UPDATE_FAIL"))
124126

125127
conf := &config.Config{
126128
Type: config.AutomationType{
@@ -132,9 +134,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
132134
_, err := automation.NewDeployAPI(client).Deploy(t.Context(), nil, "", conf)
133135
assert.Error(t, err)
134136
})
135-
t.Run("TestDeployAutomation - Scheduling Rule Upsert fails - HTTP Error", func(t *testing.T) {
137+
t.Run("TestDeployAutomation - Scheduling Rule Update fails - HTTP Error", func(t *testing.T) {
136138
client := automation.NewMockDeploySource(gomock.NewController(t))
137-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{StatusCode: 400}, nil)
139+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, api.APIError{StatusCode: 400})
138140

139141
conf := &config.Config{
140142
Type: config.AutomationType{
@@ -150,9 +152,9 @@ func TestDeployAutomation_ClientUpsertFails(t *testing.T) {
150152

151153
func TestDeployAutomation(t *testing.T) {
152154
client := automation.NewMockDeploySource(gomock.NewController(t))
153-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{
155+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{
154156
StatusCode: 200,
155-
Data: []byte("{ \"id\": \"config-id\" }"),
157+
Data: idResponseData,
156158
}, nil)
157159
conf := &config.Config{
158160
Coordinate: coordinate.Coordinate{
@@ -174,9 +176,9 @@ func TestDeployAutomation(t *testing.T) {
174176
func TestDeployAutomation_WithGivenObjectId(t *testing.T) {
175177
client := automation.NewMockDeploySource(gomock.NewController(t))
176178
objectId := "custom-object-id"
177-
client.EXPECT().Upsert(gomock.Any(), gomock.Any(), objectId, gomock.Any()).Times(1).Return(api.Response{
179+
client.EXPECT().Update(gomock.Any(), gomock.Any(), objectId, gomock.Any()).Times(1).Return(api.Response{
178180
StatusCode: 200,
179-
Data: []byte(`{ "id": "config-id" }`),
181+
Data: idResponseData,
180182
}, nil)
181183
conf := &config.Config{
182184
OriginObjectId: objectId,
@@ -191,3 +193,49 @@ func TestDeployAutomation_WithGivenObjectId(t *testing.T) {
191193
require.NoError(t, errs)
192194
assert.Equal(t, "config-id", resolvedEntity.Properties[config.IdParameter])
193195
}
196+
197+
func TestDeploy_WithCreate(t *testing.T) {
198+
client := automation.NewMockDeploySource(gomock.NewController(t))
199+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, api.APIError{StatusCode: 404})
200+
client.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{StatusCode: 200, Data: idResponseData}, nil)
201+
conf := &config.Config{
202+
Coordinate: coordinate.Coordinate{
203+
ConfigId: "config-id",
204+
},
205+
Type: config.AutomationType{
206+
Resource: config.Workflow,
207+
},
208+
}
209+
_, errs := automation.NewDeployAPI(client).Deploy(t.Context(), parameter.Properties{}, "{}", conf)
210+
assert.NoError(t, errs)
211+
}
212+
213+
func TestDeploy_FailsIfPayloadIsInvalid(t *testing.T) {
214+
client := automation.NewMockDeploySource(gomock.NewController(t))
215+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{}, api.APIError{StatusCode: 404})
216+
conf := &config.Config{
217+
Coordinate: coordinate.Coordinate{
218+
ConfigId: "config-id",
219+
},
220+
Type: config.AutomationType{
221+
Resource: config.Workflow,
222+
},
223+
}
224+
_, errs := automation.NewDeployAPI(client).Deploy(t.Context(), parameter.Properties{}, "", conf)
225+
assert.ErrorContains(t, errs, "unable to set the id field")
226+
}
227+
228+
func TestDeploy_FailsIfResponseIsMissingID(t *testing.T) {
229+
client := automation.NewMockDeploySource(gomock.NewController(t))
230+
client.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(api.Response{StatusCode: 200, Data: []byte("{}")}, nil)
231+
conf := &config.Config{
232+
Coordinate: coordinate.Coordinate{
233+
ConfigId: "config-id",
234+
},
235+
Type: config.AutomationType{
236+
Resource: config.Workflow,
237+
},
238+
}
239+
_, errs := automation.NewDeployAPI(client).Deploy(t.Context(), parameter.Properties{}, "{}", conf)
240+
assert.ErrorContains(t, errs, "id field missing")
241+
}

0 commit comments

Comments
 (0)