feat: sync add UseStateForUnknown to service_account_password_id - #98
Conversation
📝 WalkthroughWalkthroughThe pull request changes POST retry behavior for transport errors and HTTP 500 responses, adds coverage for that behavior, and adjusts cluster state handling. Terraform schemas gain plan modifiers for replacement and unknown-state preservation. Multiple data sources now fail early when unconfigured and surface malformed mapping results through diagnostics or warnings. Import IDs validate entity-specific scopes, while schema and table ownership descriptions are corrected. Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
internal/provider/data_quality_schedule_data_source.go (1)
155-180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a warning log for skipped non-map check entries.
When
checkInterfaceat Line 158 fails themap[string]interface{}type assertion, the entry is silently skipped. Other data sources in this PR (e.g.,data_quality_checks_data_source.goLine 109,groups_data_source.goLines 92, 208, 244) consistently logtflog.Warnfor unexpected entry types. Adding the same warning here would maintain consistency and aid debugging.♻️ Proposed fix
for _, checkInterface := range checks { if checkMap, ok := checkInterface.(map[string]interface{}); ok { + } else { + tflog.Warn(ctx, fmt.Sprintf("unexpected entry type, skipping: %T", checkInterface)) }internal/provider/evaluation_data_source.go (1)
139-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a warning log for skipped non-map evaluation entries.
When
evalInterfaceat Line 142 fails themap[string]interface{}type assertion, the entry is silently skipped. Other data sources in this PR consistently logtflog.Warnfor unexpected entry types. Adding the same warning here would maintain consistency and aid debugging.♻️ Proposed fix
for _, evalInterface := range evaluations { if evalMap, ok := evalInterface.(map[string]interface{}); ok { + } else { + tflog.Warn(ctx, fmt.Sprintf("unexpected entry type, skipping: %T", evalInterface)) }internal/client/client_test.go (1)
112-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest correctly verifies POST 500 non-retry; consider adding a transport-error variant.
The test validates the HTTP 500 path well. The transport-error path (line 241 in
client.go, wherec.HTTPClient.Doreturns a non-nil error) is also now non-retriable for POST but has no test coverage. Consider adding a test variant wheremockRoundTripper.RoundTripreturnsnilresponse with a non-nil error to confirm POST is not retried on transport failures either.🧪 Suggested additional test
+func (m *mockRoundTripper) withTransportError() *mockRoundTripper { + m.transportErr = fmt.Errorf("connection reset") + return m +} + +func TestPostTransportErrorNotRetried(t *testing.T) { + mock := &mockRoundTripper{statusCode: http.StatusInternalServerError, transportErr: fmt.Errorf("connection reset")} + client := &GalaxyClient{ + BaseURL: "http://localhost", + ClientID: "test", + ClientSecret: "test", + ProviderVersion: "1.0.0", + HTTPClient: &http.Client{Transport: mock}, + accessToken: "test-token", + tokenExpiry: time.Now().Add(1 * time.Hour), + } + ctx := context.Background() + var result map[string]interface{} + err := client.doRequestWithRetry(ctx, http.MethodPost, "/test", nil, &result, 3) + if err == nil { + t.Fatalf("expected error for transport failure on POST, got nil") + } + if mock.requestCount != 1 { + t.Fatalf("POST transport error should not be retried; expected 1 request, got %d", mock.requestCount) + } +}internal/provider/data_product_resource.go (1)
63-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent plan modifier assignment pattern between attributes.
catalog_idandschema_name(lines 52-56) useappendto preserve existing plan modifiers, whiledata_product_idandcreated_on(lines 63-77) use direct slice assignment (attr.PlanModifiers = []planmodifier.String{...}), which replaces any pre-existing plan modifiers. If the generated schema already defines plan modifiers for these attributes, they would be silently lost. Consider usingappendhere as well for consistency and safety.♻️ Suggested refactor for consistency
if attr, ok := s.Attributes["data_product_id"].(schema.StringAttribute); ok { - attr.PlanModifiers = []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), - } + attr.PlanModifiers = append(attr.PlanModifiers, stringplanmodifier.UseStateForUnknown()) s.Attributes["data_product_id"] = attr }And similarly for
created_on:if attr, ok := s.Attributes["created_on"].(schema.StringAttribute); ok { - attr.PlanModifiers = []planmodifier.String{ - stringplanmodifier.UseStateForUnknown(), - } + attr.PlanModifiers = append(attr.PlanModifiers, stringplanmodifier.UseStateForUnknown()) s.Attributes["created_on"] = attr }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 115f65b7-75e5-4a0c-99f1-b4748a55e222
📒 Files selected for processing (17)
docs/data-sources/schema.mddocs/data-sources/table.mdinternal/client/client.gointernal/client/client_test.gointernal/provider/cluster_resource.gointernal/provider/data_product_resource.gointernal/provider/data_quality_check_data_source.gointernal/provider/data_quality_check_resource.gointernal/provider/data_quality_checks_data_source.gointernal/provider/data_quality_schedule_data_source.gointernal/provider/datasource_schema/schema_data_source_gen.gointernal/provider/datasource_table/table_data_source_gen.gointernal/provider/evaluation_data_source.gointernal/provider/groups_data_source.gointernal/provider/role_privilege_grant_resource.gointernal/provider/service_account_password_resource.gointernal/provider/usage_example_data_source.go
Automated sync from terraform-provider-galaxy-generation repository.
Source commit: 68e2aba4b998ff84c89dd6facaf3b702e406df4f
This PR contains the latest generated provider code from the generation repository.