Skip to content

Commit 44f3e2e

Browse files
MrLujeMrLuje
authored
fix(application): prevent crash on empty sync_policy/automated block (#156)
* fix(automated): prevent crash on empty block * fix(sync_policy): ensure defaults are used only when necessary
1 parent 22c99cd commit 44f3e2e

File tree

2 files changed

+269
-49
lines changed

2 files changed

+269
-49
lines changed

‎argocd/resource_argocd_application_test.go‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,102 @@ ingress:
217217
})
218218
}
219219

220+
func TestAccArgoCDApplication_NoSyncPolicyBlock(t *testing.T) {
221+
resource.ParallelTest(t, resource.TestCase{
222+
PreCheck: func() { testAccPreCheck(t) },
223+
ProviderFactories: testAccProviders,
224+
Steps: []resource.TestStep{
225+
{
226+
Config: testAccArgoCDApplicationNoSyncPolicy(acctest.RandomWithPrefix("test-acc")),
227+
Check: resource.ComposeTestCheckFunc(
228+
resource.TestCheckResourceAttrSet(
229+
"argocd_application.simple",
230+
"metadata.0.uid",
231+
),
232+
resource.TestCheckNoResourceAttr(
233+
"argocd_application.simple",
234+
"spec.0.sync_policy.0.retry.0.backoff.duration",
235+
),
236+
resource.TestCheckNoResourceAttr(
237+
"argocd_application.simple",
238+
"spec.0.sync_policy.0.automated.prune",
239+
),
240+
),
241+
},
242+
}})
243+
}
244+
245+
func TestAccArgoCDApplication_EmptySyncPolicyBlock(t *testing.T) {
246+
resource.ParallelTest(t, resource.TestCase{
247+
PreCheck: func() { testAccPreCheck(t) },
248+
ProviderFactories: testAccProviders,
249+
Steps: []resource.TestStep{
250+
{
251+
Config: testAccArgoCDApplicationEmptySyncPolicy(acctest.RandomWithPrefix("test-acc")),
252+
Check: resource.ComposeTestCheckFunc(
253+
resource.TestCheckResourceAttrSet(
254+
"argocd_application.simple",
255+
"metadata.0.uid",
256+
),
257+
resource.TestCheckNoResourceAttr(
258+
"argocd_application.simple",
259+
"spec.0.sync_policy.0.retry.0.backoff.duration",
260+
),
261+
resource.TestCheckNoResourceAttr(
262+
"argocd_application.simple",
263+
"spec.0.sync_policy.0.automated.prune",
264+
),
265+
),
266+
},
267+
}})
268+
}
269+
270+
func TestAccArgoCDApplication_NoAutomatedBlock(t *testing.T) {
271+
resource.ParallelTest(t, resource.TestCase{
272+
PreCheck: func() { testAccPreCheck(t) },
273+
ProviderFactories: testAccProviders,
274+
Steps: []resource.TestStep{
275+
{
276+
Config: testAccArgoCDApplicationNoAutomated(acctest.RandomWithPrefix("test-acc")),
277+
Check: resource.ComposeTestCheckFunc(
278+
resource.TestCheckResourceAttrSet(
279+
"argocd_application.simple",
280+
"metadata.0.uid",
281+
),
282+
resource.TestCheckResourceAttrSet(
283+
"argocd_application.simple",
284+
"spec.0.sync_policy.0.retry.0.backoff.duration",
285+
),
286+
resource.TestCheckNoResourceAttr(
287+
"argocd_application.simple",
288+
"spec.0.sync_policy.0.automated.prune",
289+
),
290+
),
291+
},
292+
}})
293+
}
294+
295+
func TestAccArgoCDApplication_EmptyAutomatedBlock(t *testing.T) {
296+
resource.ParallelTest(t, resource.TestCase{
297+
PreCheck: func() { testAccPreCheck(t) },
298+
ProviderFactories: testAccProviders,
299+
Steps: []resource.TestStep{
300+
{
301+
Config: testAccArgoCDApplicationEmptyAutomated(acctest.RandomWithPrefix("test-acc")),
302+
Check: resource.ComposeTestCheckFunc(
303+
resource.TestCheckResourceAttrSet(
304+
"argocd_application.simple",
305+
"metadata.0.uid",
306+
),
307+
resource.TestCheckNoResourceAttr(
308+
"argocd_application.simple",
309+
"spec.0.sync_policy.0.automated.prune",
310+
),
311+
),
312+
},
313+
}})
314+
}
315+
220316
func testAccArgoCDApplicationSimple(name string) string {
221317
return fmt.Sprintf(`
222318
resource "argocd_application" "simple" {
@@ -567,6 +663,121 @@ resource "argocd_application" "ignore_differences_jqpe" {
567663
`, name)
568664
}
569665

666+
func testAccArgoCDApplicationNoSyncPolicy(name string) string {
667+
return fmt.Sprintf(`
668+
resource "argocd_application" "simple" {
669+
metadata {
670+
name = "%s"
671+
namespace = "argocd"
672+
}
673+
spec {
674+
source {
675+
repo_url = "https://charts.bitnami.com/bitnami"
676+
chart = "redis"
677+
target_revision = "15.3.0"
678+
helm {
679+
release_name = "testing"
680+
}
681+
}
682+
destination {
683+
server = "https://kubernetes.default.svc"
684+
namespace = "default"
685+
}
686+
}
687+
}
688+
`, name)
689+
}
690+
691+
func testAccArgoCDApplicationEmptySyncPolicy(name string) string {
692+
return fmt.Sprintf(`
693+
resource "argocd_application" "simple" {
694+
metadata {
695+
name = "%s"
696+
namespace = "argocd"
697+
}
698+
spec {
699+
source {
700+
repo_url = "https://charts.bitnami.com/bitnami"
701+
chart = "redis"
702+
target_revision = "15.3.0"
703+
helm {
704+
release_name = "testing"
705+
}
706+
}
707+
sync_policy {
708+
}
709+
destination {
710+
server = "https://kubernetes.default.svc"
711+
namespace = "default"
712+
}
713+
}
714+
}
715+
`, name)
716+
}
717+
718+
func testAccArgoCDApplicationNoAutomated(name string) string {
719+
return fmt.Sprintf(`
720+
resource "argocd_application" "simple" {
721+
metadata {
722+
name = "%s"
723+
namespace = "argocd"
724+
}
725+
spec {
726+
source {
727+
repo_url = "https://charts.bitnami.com/bitnami"
728+
chart = "redis"
729+
target_revision = "15.3.0"
730+
helm {
731+
release_name = "testing"
732+
}
733+
}
734+
sync_policy {
735+
retry {
736+
limit = "5"
737+
backoff = {
738+
duration = "30s"
739+
max_duration = "2m"
740+
factor = "2"
741+
}
742+
}
743+
}
744+
destination {
745+
server = "https://kubernetes.default.svc"
746+
namespace = "default"
747+
}
748+
}
749+
}
750+
`, name)
751+
}
752+
753+
func testAccArgoCDApplicationEmptyAutomated(name string) string {
754+
return fmt.Sprintf(`
755+
resource "argocd_application" "simple" {
756+
metadata {
757+
name = "%s"
758+
namespace = "argocd"
759+
}
760+
spec {
761+
source {
762+
repo_url = "https://charts.bitnami.com/bitnami"
763+
chart = "redis"
764+
target_revision = "15.3.0"
765+
helm {
766+
release_name = "testing"
767+
}
768+
}
769+
sync_policy {
770+
automated = {}
771+
}
772+
destination {
773+
server = "https://kubernetes.default.svc"
774+
namespace = "default"
775+
}
776+
}
777+
}
778+
`, name)
779+
}
780+
570781
func testAccSkipFeatureIgnoreDiffJQPathExpressions() (bool, error) {
571782
p, _ := testAccProviders["argocd"]()
572783
_ = p.Configure(context.Background(), &terraform.ResourceConfig{})

‎argocd/structure_application.go‎

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -257,78 +257,87 @@ func expandApplicationSyncPolicy(_sp []interface{}) (*application.SyncPolicy, di
257257
return nil, nil
258258
}
259259
sp := _sp[0]
260+
if sp == nil {
261+
return &application.SyncPolicy{}, nil
262+
}
260263
var automated = &application.SyncPolicyAutomated{}
261264
var syncOptions application.SyncOptions
262265
var retry = &application.RetryStrategy{}
266+
var syncPolicy = &application.SyncPolicy{}
263267

264-
if a, ok := sp.(map[string]interface{})["automated"]; ok {
265-
for k, v := range a.(map[string]interface{}) {
266-
if k == "prune" {
267-
automated.Prune = v.(bool)
268-
}
269-
if k == "self_heal" {
270-
automated.SelfHeal = v.(bool)
271-
}
272-
if k == "allow_empty" {
273-
automated.AllowEmpty = v.(bool)
268+
if a, ok := sp.(map[string]interface{})["automated"].(map[string]interface{}); ok {
269+
if len(a) > 0 {
270+
for k, v := range a {
271+
if k == "prune" {
272+
automated.Prune = v.(bool)
273+
}
274+
if k == "self_heal" {
275+
automated.SelfHeal = v.(bool)
276+
}
277+
if k == "allow_empty" {
278+
automated.AllowEmpty = v.(bool)
279+
}
274280
}
281+
syncPolicy.Automated = automated
275282
}
276283
}
277284
if v, ok := sp.(map[string]interface{})["sync_options"]; ok {
278285
sOpts := v.([]interface{})
279-
for _, sOpt := range sOpts {
280-
syncOptions = append(syncOptions, sOpt.(string))
286+
if len(sOpts) > 0 {
287+
for _, sOpt := range sOpts {
288+
syncOptions = append(syncOptions, sOpt.(string))
289+
}
290+
syncPolicy.SyncOptions = syncOptions
281291
}
282292
}
283293
if _retry, ok := sp.(map[string]interface{})["retry"].([]interface{}); ok {
284294
if len(_retry) > 0 {
285-
r := _retry[0]
286-
for k, v := range r.(map[string]interface{}) {
287-
if k == "limit" {
288-
var err error
289-
retry.Limit, err = convertStringToInt64(v.(string))
290-
if err != nil {
291-
return nil, []diag.Diagnostic{
292-
{
293-
Severity: diag.Error,
294-
Summary: "Error converting retry limit to integer",
295-
Detail: err.Error(),
296-
},
295+
r := (_retry[0]).(map[string]interface{})
296+
if len(r) > 0 {
297+
for k, v := range r {
298+
if k == "limit" {
299+
var err error
300+
retry.Limit, err = convertStringToInt64(v.(string))
301+
if err != nil {
302+
return nil, []diag.Diagnostic{
303+
{
304+
Severity: diag.Error,
305+
Summary: "Error converting retry limit to integer",
306+
Detail: err.Error(),
307+
},
308+
}
297309
}
298310
}
299-
}
300-
if k == "backoff" {
301-
retry.Backoff = &application.Backoff{}
302-
for kb, vb := range v.(map[string]interface{}) {
303-
if kb == "duration" {
304-
retry.Backoff.Duration = vb.(string)
305-
}
306-
if kb == "max_duration" {
307-
retry.Backoff.MaxDuration = vb.(string)
308-
}
309-
if kb == "factor" {
310-
factor, err := convertStringToInt64Pointer(vb.(string))
311-
if err != nil {
312-
return nil, []diag.Diagnostic{
313-
{
314-
Severity: diag.Error,
315-
Summary: "Error converting backoff factor to integer",
316-
Detail: err.Error(),
317-
},
311+
if k == "backoff" {
312+
retry.Backoff = &application.Backoff{}
313+
for kb, vb := range v.(map[string]interface{}) {
314+
if kb == "duration" {
315+
retry.Backoff.Duration = vb.(string)
316+
}
317+
if kb == "max_duration" {
318+
retry.Backoff.MaxDuration = vb.(string)
319+
}
320+
if kb == "factor" {
321+
factor, err := convertStringToInt64Pointer(vb.(string))
322+
if err != nil {
323+
return nil, []diag.Diagnostic{
324+
{
325+
Severity: diag.Error,
326+
Summary: "Error converting backoff factor to integer",
327+
Detail: err.Error(),
328+
},
329+
}
318330
}
331+
retry.Backoff.Factor = factor
319332
}
320-
retry.Backoff.Factor = factor
321333
}
322334
}
323335
}
336+
syncPolicy.Retry = retry
324337
}
325338
}
326339
}
327-
return &application.SyncPolicy{
328-
Automated: automated,
329-
SyncOptions: syncOptions,
330-
Retry: retry,
331-
}, nil
340+
return syncPolicy, nil
332341
}
333342

334343
func expandApplicationIgnoreDifferences(ids []interface{}) (

0 commit comments

Comments
 (0)