fix: accept day-name strings in SystemDayOfWeek unmarshal (v24 + v25)#36
Open
spbsoluble wants to merge 2 commits into
Open
fix: accept day-name strings in SystemDayOfWeek unmarshal (v24 + v25)#36spbsoluble wants to merge 2 commits into
spbsoluble wants to merge 2 commits into
Conversation
Keyfactor Command serializes WeeklyModel.Days as day-name strings (e.g. "Monday") in some API responses, but SystemDayOfWeek.UnmarshalJSON only accepted JSON integers, causing Weekly-scheduled resources to fail to deserialize. Try the integer form first, preserving existing enum validation, and fall back to the existing Parse() day-name mapping when the payload is a JSON string. Malformed strings and out-of-range ints still error clearly. Applies to both v1 and v2 API packages in this module.
Fixes keyfactor-pub/terraform-provider-keyfactor#185: Keyfactor Command v25.5 serializes WeeklyModel.Days as day-name strings (e.g. "Monday") in GET /CertificateAuthority responses, but SystemDayOfWeek.UnmarshalJSON only accepted JSON integers, causing any Weekly-scheduled CA to fail to deserialize. Try the integer form first, preserving existing enum validation, and fall back to the existing Parse() day-name mapping when the payload is a JSON string. Malformed strings and out-of-range ints still error clearly. Applies to both v1 and v2 API packages in this module.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
SystemDayOfWeek.UnmarshalJSONonly accepts a JSON integer, but Keyfactor Command (observed on v25.5) serializesWeeklyModel.Daysas day-name strings (e.g.["Monday"]) inGET /CertificateAuthorityresponses. Any Weekly-shaped CA schedule therefore fails to deserialize, which breaks every subsequent Read/Update of that CA in downstream consumers.Tracked downstream as keyfactor-pub/terraform-provider-keyfactor#185.
Fix
UnmarshalJSONnow tries the integer form first (preserving the original validation againstAllowedSystemDayOfWeekEnumValuesand its error message), then falls back to the string form via the existingParse()day-name mapper. A payload that is neither a valid JSON integer nor a JSON string returns a clear error.Applied identically to all four generated copies:
v24/api/keyfactor/{v1,v2}andv25/api/keyfactor/{v1,v2}.Verification
WeeklyModelround-trips for both{"Days":["Monday","Friday"],...}and int-index forms.json: cannot unmarshal string into Go value of type int32.go test ./...green in both the v24 and v25 modules (with-vet=offdue to a pre-existing, unrelatedfmt.Errorfnon-constant-format vet error inconfiguration.goonmain).Notes
feat/sdk-v25-migrationbranch generates this file from the sharedcustom-templates/go/model_enum.mustache, which still has the int-only unmarshal — that template will need the equivalent fallback when the migration branch is picked back up. That template is shared by ~80+ int-backed enums (all of which have unusedParse()string helpers), so a blanket template change deserves its own discussion; this PR deliberately fixes only the enum with an observed wire regression.