Added support for managedFieldsManagers and Helm templating version #400
Conversation
tonedefdev
force-pushed
the
add-managed-fields-managers-helm-version
branch
from
5c39154 to
2a149f5
Compare
|
This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
... |
Anything else needed @onematchfox to get this approved? |
|
This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
... |
|
This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
There was a problem hiding this comment.
Thanks for this PR and your patience in me getting to it. Please see comments inline. Also, please could you ensure that these fields are added to the tests:
versioncan be added toTestAccArgoCDApplication_Helmmanaged_field_managerscan be added toTestAccArgoCDApplication_IgnoreDifferences
| @@ -311,6 +311,10 @@ func expandApplicationSourceHelm(in []interface{}) *application.ApplicationSourc | |||
| result.SkipCrds = v.(bool) | |||
| } | |||
|
|
|||
| if v, ok := a["version"]; ok { | |||
There was a problem hiding this comment.
This code ensures that the version is correctly set when creating the application in ArgoCD however, you are missing the oppositye side of this code which ensures that the Terraform state is updated to reflect the current state of the application in ArgoCD. See flattenApplicationSourceHelm.
| if v, ok := id["managed_fields_managers"]; ok { | ||
| managedFieldsManagers := v.(*schema.Set).List() | ||
| for _, fieldsManager := range managedFieldsManagers { | ||
| elem.ManagedFieldsManagers = append(elem.ManagedFieldsManagers, fieldsManager.(string)) |
There was a problem hiding this comment.
Same issue here. See flattenApplicationIgnoreDifferences
…ferences for external controllers that manage fields. Added support for adding the helm version for templating Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
…Added test cases for helm version and managed fields managers Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
tonedefdev
force-pushed
the
add-managed-fields-managers-helm-version
branch
from
66db8b8 to
adadffb
Compare
Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
…epted values for helm version Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
…verification in tests Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
| ignore_difference { | ||
| group = "apps" | ||
| kind = "Deployment" | ||
| json_pointers = ["/spec/replicas"] |
There was a problem hiding this comment.
| json_pointers = ["/spec/replicas"] | |
| json_pointers = ["/spec/replicas"] |
nit: mixed tabs/spaces (not your fault these tests are painful in this regard - spaces in *.go files 🤮 )
| json_pointers = [ | ||
| "/spec/replicas", | ||
| "/spec/template/spec/metadata/labels/somelabel", | ||
| ] | ||
| managed_fields_managers = [ | ||
| "some-controller-owner", | ||
| "some-other-controller-owner", | ||
| ] |
There was a problem hiding this comment.
| json_pointers = [ | |
| "/spec/replicas", | |
| "/spec/template/spec/metadata/labels/somelabel", | |
| ] | |
| managed_fields_managers = [ | |
| "some-controller-owner", | |
| "some-other-controller-owner", | |
| ] | |
| managed_fields_managers = [ | |
| "some-controller-owner", | |
| "some-other-controller-owner", | |
| ] |
Good to check that everything works both with and without json_pointers
| ResourceName: "argocd_application.ignore_differences_managed_fields_managers", | ||
| ImportState: true, | ||
| ImportStateVerify: true, | ||
| ImportStateVerifyIgnore: []string{"wait", "cascade", "status"}, |
There was a problem hiding this comment.
| ImportStateVerifyIgnore: []string{"wait", "cascade", "status"}, | |
| ImportStateVerifyIgnore: []string{"wait", "cascade", "status", "validate"}, |
Need to add validate in here to fix tests after #411 was merged
There was a problem hiding this comment.
I was wondering if this was what I needed to fix the tests! Thank you. I was struggling to figure out where validate was coming from. When I was building the provider on local I saw no trace of this, so thanks for clearing this up!
… Removed json pointers on one of the tests Signed-off-by: Tony Owens <anthony.owens1987@gmail.com>
@onematchfox - all tests are passing now and I made the requested changes! Hopefully we're good to merge now, but if we need more changes, I'm happy to oblige |
Closes #396