From 3cc5ccd37c1745e9320c92f39455d3092861daf3 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Wed, 29 Apr 2026 19:01:44 +0530 Subject: [PATCH 1/4] Fix preferred_api_document_source dropped without api_document_path (#14748) --- opslevel/resource_opslevel_service.go | 69 ++++++------- opslevel/resource_opslevel_service_test.go | 108 +++++++++++++++++++++ tests/service.tftest.hcl | 23 +++++ 3 files changed, 162 insertions(+), 38 deletions(-) create mode 100644 opslevel/resource_opslevel_service_test.go diff --git a/opslevel/resource_opslevel_service.go b/opslevel/resource_opslevel_service.go index a242493a..071ebd45 100644 --- a/opslevel/resource_opslevel_service.go +++ b/opslevel/resource_opslevel_service.go @@ -309,19 +309,10 @@ func (r *ServiceResource) Create(ctx context.Context, req resource.CreateRequest return } - if planModel.ApiDocumentPath.ValueString() != "" { - apiDocPath := planModel.ApiDocumentPath.ValueString() - if planModel.PreferredApiDocumentSource.IsNull() { - if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, nil); err != nil { - resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to set provided 'api_document_path' %s for service. error: %s", apiDocPath, err)) - return - } - } else { - sourceEnum := opslevel.ApiDocumentSourceEnum(planModel.PreferredApiDocumentSource.ValueString()) - if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, &sourceEnum); err != nil { - resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to set provided 'api_document_path' %s with doc source '%s' for service. error: %s", apiDocPath, sourceEnum, err)) - return - } + if shouldUpdate, apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel, nil); shouldUpdate { + if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, sourceEnum); err != nil { + resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to update API document settings for service %s. error: %s", service.Name, err)) + return } } @@ -401,6 +392,30 @@ func unsetIdentifierHelper(plan, state basetypes.StringValue) *opslevel.Identifi return nil } +func serviceApiDocSettingsUpdateInput(plan ServiceResourceModel, state *ServiceResourceModel) (bool, string, *opslevel.ApiDocumentSourceEnum) { + managesApiDocPath := !plan.ApiDocumentPath.IsNull() + managesApiDocSource := !plan.PreferredApiDocumentSource.IsNull() + if state != nil { + managesApiDocPath = managesApiDocPath || !state.ApiDocumentPath.IsNull() + managesApiDocSource = managesApiDocSource || !state.PreferredApiDocumentSource.IsNull() + } + if !managesApiDocPath && !managesApiDocSource { + return false, "", nil + } + + apiDocPath := "" + if !plan.ApiDocumentPath.IsNull() { + apiDocPath = plan.ApiDocumentPath.ValueString() + } + + if plan.PreferredApiDocumentSource.IsNull() { + return true, apiDocPath, nil + } + + sourceEnum := opslevel.ApiDocumentSourceEnum(plan.PreferredApiDocumentSource.ValueString()) + return true, apiDocPath, &sourceEnum +} + func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { planModel := read[ServiceResourceModel](ctx, &resp.Diagnostics, req.Plan) stateModel := read[ServiceResourceModel](ctx, &resp.Diagnostics, req.State) @@ -477,33 +492,11 @@ func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest } } - if planModel.ApiDocumentPath.IsNull() { - if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), "", nil); err != nil { - resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to unset 'api_document_path' for service %s. error: %s", service.Name, err)) + if shouldUpdate, apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel, &stateModel); shouldUpdate { + if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, sourceEnum); err != nil { + resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to update API document settings for service %s. error: %s", service.Name, err)) return } - } else { - apiDocPath := planModel.ApiDocumentPath.ValueString() - if planModel.PreferredApiDocumentSource.IsNull() { - if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, nil); err != nil { - resp.Diagnostics.AddError("opslevel client error", - fmt.Sprintf( - "Unable to set provided 'api_document_path' %s for service. error: %s", - apiDocPath, err), - ) - return - } - } else { - sourceEnum := opslevel.ApiDocumentSourceEnum(planModel.PreferredApiDocumentSource.ValueString()) - if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, &sourceEnum); err != nil { - resp.Diagnostics.AddError("opslevel client error", - fmt.Sprintf( - "Unable to set provided 'api_document_path' %s with doc source '%s' for service. error: %s", - apiDocPath, sourceEnum, err), - ) - return - } - } } // fetch the service again, since other mutations are performed after the create/update step diff --git a/opslevel/resource_opslevel_service_test.go b/opslevel/resource_opslevel_service_test.go new file mode 100644 index 00000000..a9634fd6 --- /dev/null +++ b/opslevel/resource_opslevel_service_test.go @@ -0,0 +1,108 @@ +package opslevel + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-framework/types" + opslevelgo "github.com/opslevel/opslevel-go/v2026" +) + +func TestServiceApiDocSettingsUpdateInput(t *testing.T) { + testCases := []struct { + name string + plan ServiceResourceModel + state *ServiceResourceModel + expectedUpdate bool + expectedDocPath string + expectedDocSource *opslevelgo.ApiDocumentSourceEnum + }{ + { + name: "create ignores unmanaged settings", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringNull(), + }, + expectedUpdate: false, + }, + { + name: "create updates push source without path", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), + }, + expectedUpdate: true, + expectedDocPath: "", + expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPush, + }, + { + name: "create updates pull source without path", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), + }, + expectedUpdate: true, + expectedDocPath: "", + expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPull, + }, + { + name: "create updates path without source", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringValue("openapi.yaml"), + PreferredApiDocumentSource: types.StringNull(), + }, + expectedUpdate: true, + expectedDocPath: "openapi.yaml", + }, + { + name: "update clears managed source", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringNull(), + }, + state: &ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), + }, + expectedUpdate: true, + expectedDocPath: "", + }, + { + name: "update clears managed path", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringNull(), + }, + state: &ServiceResourceModel{ + ApiDocumentPath: types.StringValue("openapi.yaml"), + PreferredApiDocumentSource: types.StringNull(), + }, + expectedUpdate: true, + expectedDocPath: "", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + shouldUpdate, docPath, docSource := serviceApiDocSettingsUpdateInput(testCase.plan, testCase.state) + + if shouldUpdate != testCase.expectedUpdate { + t.Fatalf("expected update %t, got %t", testCase.expectedUpdate, shouldUpdate) + } + if docPath != testCase.expectedDocPath { + t.Fatalf("expected doc path %q, got %q", testCase.expectedDocPath, docPath) + } + if testCase.expectedDocSource == nil { + if docSource != nil { + t.Fatalf("expected nil doc source, got %q", *docSource) + } + return + } + if docSource == nil { + t.Fatalf("expected doc source %q, got nil", *testCase.expectedDocSource) + } + if *docSource != *testCase.expectedDocSource { + t.Fatalf("expected doc source %q, got %q", *testCase.expectedDocSource, *docSource) + } + }) + } +} diff --git a/tests/service.tftest.hcl b/tests/service.tftest.hcl index 0ef0134c..dc58a465 100644 --- a/tests/service.tftest.hcl +++ b/tests/service.tftest.hcl @@ -238,6 +238,29 @@ run "resource_service_create_with_empty_optional_fields" { } +run "resource_service_create_with_preferred_api_document_source_without_path" { + + variables { + name = "New ${var.name} with API doc source only" + preferred_api_document_source = "PUSH" + } + + module { + source = "./opslevel_modules/modules/service" + } + + assert { + condition = opslevel_service.this.api_document_path == null + error_message = var.error_expected_null_field + } + + assert { + condition = opslevel_service.this.preferred_api_document_source == "PUSH" + error_message = "expected preferred_api_document_source to be persisted without api_document_path" + } + +} + run "resource_service_update_unset_optional_fields" { variables { From 23e82ef5275963773ecfd6e56186770f70814d2d Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Wed, 29 Apr 2026 19:03:06 +0530 Subject: [PATCH 2/4] add changie --- .changes/unreleased/Fixed-20260429-190243.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changes/unreleased/Fixed-20260429-190243.yaml diff --git a/.changes/unreleased/Fixed-20260429-190243.yaml b/.changes/unreleased/Fixed-20260429-190243.yaml new file mode 100644 index 00000000..1f0e9c22 --- /dev/null +++ b/.changes/unreleased/Fixed-20260429-190243.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: Fixed `opslevel_service` dropping `preferred_api_document_source` after apply when `api_document_path` was not set, which caused a "Provider produced inconsistent result after apply" error +time: 2026-04-29T19:02:43.236755+05:30 From 5f3e072d4aad95de67b17423278a12274a9e4b3a Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Thu, 30 Apr 2026 16:43:16 +0530 Subject: [PATCH 3/4] Skip API doc settings update when plan equals state --- opslevel/resource_opslevel_service.go | 6 ++++ opslevel/resource_opslevel_service_test.go | 38 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/opslevel/resource_opslevel_service.go b/opslevel/resource_opslevel_service.go index 071ebd45..fd2b200c 100644 --- a/opslevel/resource_opslevel_service.go +++ b/opslevel/resource_opslevel_service.go @@ -393,6 +393,12 @@ func unsetIdentifierHelper(plan, state basetypes.StringValue) *opslevel.Identifi } func serviceApiDocSettingsUpdateInput(plan ServiceResourceModel, state *ServiceResourceModel) (bool, string, *opslevel.ApiDocumentSourceEnum) { + if state != nil && + plan.ApiDocumentPath.Equal(state.ApiDocumentPath) && + plan.PreferredApiDocumentSource.Equal(state.PreferredApiDocumentSource) { + return false, "", nil + } + managesApiDocPath := !plan.ApiDocumentPath.IsNull() managesApiDocSource := !plan.PreferredApiDocumentSource.IsNull() if state != nil { diff --git a/opslevel/resource_opslevel_service_test.go b/opslevel/resource_opslevel_service_test.go index a9634fd6..21f398c4 100644 --- a/opslevel/resource_opslevel_service_test.go +++ b/opslevel/resource_opslevel_service_test.go @@ -66,6 +66,44 @@ func TestServiceApiDocSettingsUpdateInput(t *testing.T) { expectedUpdate: true, expectedDocPath: "", }, + { + name: "update ignores unchanged source without path", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), + }, + state: &ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), + }, + expectedUpdate: false, + }, + { + name: "update ignores unchanged path and source", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringValue("openapi.yaml"), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), + }, + state: &ServiceResourceModel{ + ApiDocumentPath: types.StringValue("openapi.yaml"), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), + }, + expectedUpdate: false, + }, + { + name: "update changes managed source without path", + plan: ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), + }, + state: &ServiceResourceModel{ + ApiDocumentPath: types.StringNull(), + PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), + }, + expectedUpdate: true, + expectedDocPath: "", + expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPush, + }, { name: "update clears managed path", plan: ServiceResourceModel{ From d7829302a12f7b5a62f958f27b383aa0a242c262 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 1 May 2026 00:19:42 +0530 Subject: [PATCH 4/4] Simplify API doc settings helper (#647 review) --- opslevel/resource_opslevel_service.go | 49 +++++--------- opslevel/resource_opslevel_service_test.go | 79 +++------------------- 2 files changed, 25 insertions(+), 103 deletions(-) diff --git a/opslevel/resource_opslevel_service.go b/opslevel/resource_opslevel_service.go index fd2b200c..b68934fb 100644 --- a/opslevel/resource_opslevel_service.go +++ b/opslevel/resource_opslevel_service.go @@ -309,7 +309,8 @@ func (r *ServiceResource) Create(ctx context.Context, req resource.CreateRequest return } - if shouldUpdate, apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel, nil); shouldUpdate { + if !planModel.ApiDocumentPath.IsNull() || !planModel.PreferredApiDocumentSource.IsNull() { + apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel) if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, sourceEnum); err != nil { resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to update API document settings for service %s. error: %s", service.Name, err)) return @@ -392,36 +393,6 @@ func unsetIdentifierHelper(plan, state basetypes.StringValue) *opslevel.Identifi return nil } -func serviceApiDocSettingsUpdateInput(plan ServiceResourceModel, state *ServiceResourceModel) (bool, string, *opslevel.ApiDocumentSourceEnum) { - if state != nil && - plan.ApiDocumentPath.Equal(state.ApiDocumentPath) && - plan.PreferredApiDocumentSource.Equal(state.PreferredApiDocumentSource) { - return false, "", nil - } - - managesApiDocPath := !plan.ApiDocumentPath.IsNull() - managesApiDocSource := !plan.PreferredApiDocumentSource.IsNull() - if state != nil { - managesApiDocPath = managesApiDocPath || !state.ApiDocumentPath.IsNull() - managesApiDocSource = managesApiDocSource || !state.PreferredApiDocumentSource.IsNull() - } - if !managesApiDocPath && !managesApiDocSource { - return false, "", nil - } - - apiDocPath := "" - if !plan.ApiDocumentPath.IsNull() { - apiDocPath = plan.ApiDocumentPath.ValueString() - } - - if plan.PreferredApiDocumentSource.IsNull() { - return true, apiDocPath, nil - } - - sourceEnum := opslevel.ApiDocumentSourceEnum(plan.PreferredApiDocumentSource.ValueString()) - return true, apiDocPath, &sourceEnum -} - func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { planModel := read[ServiceResourceModel](ctx, &resp.Diagnostics, req.Plan) stateModel := read[ServiceResourceModel](ctx, &resp.Diagnostics, req.State) @@ -498,7 +469,9 @@ func (r *ServiceResource) Update(ctx context.Context, req resource.UpdateRequest } } - if shouldUpdate, apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel, &stateModel); shouldUpdate { + if !planModel.ApiDocumentPath.Equal(stateModel.ApiDocumentPath) || + !planModel.PreferredApiDocumentSource.Equal(stateModel.PreferredApiDocumentSource) { + apiDocPath, sourceEnum := serviceApiDocSettingsUpdateInput(planModel) if _, err := r.client.ServiceApiDocSettingsUpdate(string(service.Id), apiDocPath, sourceEnum); err != nil { resp.Diagnostics.AddError("opslevel client error", fmt.Sprintf("Unable to update API document settings for service %s. error: %s", service.Name, err)) return @@ -560,3 +533,15 @@ func updateServiceNote(client opslevel.Client, service opslevel.Service, planMod return client.UpdateServiceNote(serviceNoteUpdateInput) } + +func serviceApiDocSettingsUpdateInput(plan ServiceResourceModel) (string, *opslevel.ApiDocumentSourceEnum) { + apiDocPath := "" + if !plan.ApiDocumentPath.IsNull() { + apiDocPath = plan.ApiDocumentPath.ValueString() + } + if plan.PreferredApiDocumentSource.IsNull() { + return apiDocPath, nil + } + sourceEnum := opslevel.ApiDocumentSourceEnum(plan.PreferredApiDocumentSource.ValueString()) + return apiDocPath, &sourceEnum +} diff --git a/opslevel/resource_opslevel_service_test.go b/opslevel/resource_opslevel_service_test.go index 21f398c4..ebe0ac2d 100644 --- a/opslevel/resource_opslevel_service_test.go +++ b/opslevel/resource_opslevel_service_test.go @@ -11,121 +11,58 @@ func TestServiceApiDocSettingsUpdateInput(t *testing.T) { testCases := []struct { name string plan ServiceResourceModel - state *ServiceResourceModel - expectedUpdate bool expectedDocPath string expectedDocSource *opslevelgo.ApiDocumentSourceEnum }{ { - name: "create ignores unmanaged settings", + name: "neither field set", plan: ServiceResourceModel{ ApiDocumentPath: types.StringNull(), PreferredApiDocumentSource: types.StringNull(), }, - expectedUpdate: false, + expectedDocPath: "", }, { - name: "create updates push source without path", + name: "push source without path", plan: ServiceResourceModel{ ApiDocumentPath: types.StringNull(), PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), }, - expectedUpdate: true, expectedDocPath: "", expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPush, }, { - name: "create updates pull source without path", + name: "pull source without path", plan: ServiceResourceModel{ ApiDocumentPath: types.StringNull(), PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), }, - expectedUpdate: true, expectedDocPath: "", expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPull, }, { - name: "create updates path without source", + name: "path without source", plan: ServiceResourceModel{ ApiDocumentPath: types.StringValue("openapi.yaml"), PreferredApiDocumentSource: types.StringNull(), }, - expectedUpdate: true, expectedDocPath: "openapi.yaml", }, { - name: "update clears managed source", - plan: ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringNull(), - }, - state: &ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), - }, - expectedUpdate: true, - expectedDocPath: "", - }, - { - name: "update ignores unchanged source without path", + name: "path with source", plan: ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), - }, - state: &ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), - }, - expectedUpdate: false, - }, - { - name: "update ignores unchanged path and source", - plan: ServiceResourceModel{ - ApiDocumentPath: types.StringValue("openapi.yaml"), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), - }, - state: &ServiceResourceModel{ ApiDocumentPath: types.StringValue("openapi.yaml"), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), - }, - expectedUpdate: false, - }, - { - name: "update changes managed source without path", - plan: ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPush)), }, - state: &ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringValue(string(opslevelgo.ApiDocumentSourceEnumPull)), - }, - expectedUpdate: true, - expectedDocPath: "", + expectedDocPath: "openapi.yaml", expectedDocSource: &opslevelgo.ApiDocumentSourceEnumPush, }, - { - name: "update clears managed path", - plan: ServiceResourceModel{ - ApiDocumentPath: types.StringNull(), - PreferredApiDocumentSource: types.StringNull(), - }, - state: &ServiceResourceModel{ - ApiDocumentPath: types.StringValue("openapi.yaml"), - PreferredApiDocumentSource: types.StringNull(), - }, - expectedUpdate: true, - expectedDocPath: "", - }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - shouldUpdate, docPath, docSource := serviceApiDocSettingsUpdateInput(testCase.plan, testCase.state) + docPath, docSource := serviceApiDocSettingsUpdateInput(testCase.plan) - if shouldUpdate != testCase.expectedUpdate { - t.Fatalf("expected update %t, got %t", testCase.expectedUpdate, shouldUpdate) - } if docPath != testCase.expectedDocPath { t.Fatalf("expected doc path %q, got %q", testCase.expectedDocPath, docPath) }