Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include RequestTimeout in marshal/unmarshal of ServiceResolverConfigE… #19031

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

cthain
Copy link
Contributor

@cthain cthain commented Sep 29, 2023

Description

This PR fixes a bug where we’re missing the custom JSON marshal and unmarshal for the ServiceResolverConfigEntry.RequestTimeout field. This has the effect that the config entry can be written successfully to Consul but it cannot be read back through the API.

Testing & Reproduction steps

Before the changes proposed in this PR:

$ consul agent -dev &> /dev/null &
$ consul config write -
Kind      = "service-resolver"
Name      = "frontend-se"
ConnectTimeout = "10s"
RequestTimeout = "15s"
Subsets = {
 v1 = {
   OnlyPassing = true
 }
 v2  = {
   OnlyPassing = true
 }
}^D
Config entry written: service-resolver/frontend-se
$ consul config read -kind service-resolver -name frontend-se
Error reading config entry service-resolver/frontend-se: json: cannot unmarshal string into Go struct field .RequestTimeout of type time.Duration

After these changes:

# setup as above
consul config read -kind service-resolver -name frontend-se
{
    "ConnectTimeout": "10s",
    "RequestTimeout": "15s",
    "Kind": "service-resolver",
    "Name": "frontend-se",
    "Partition": "default",
    "Namespace": "default",
    "Subsets": {
        "v1": {
            "OnlyPassing": true
        },
        "v2": {
            "OnlyPassing": true
        }
    },
    "CreateIndex": 22,
    "ModifyIndex": 22
}

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Sep 29, 2023
@cthain cthain requested review from roncodingenthusiast and a team September 29, 2023 17:07
@cthain cthain self-assigned this Sep 29, 2023
@cthain cthain added backport/1.14 backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. labels Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. theme/api Relating to the HTTP API interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants