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

Backport of Include RequestTimeout in marshal/unmarshal of ServiceResolverConfigE… into release/1.16.x #19034

Conversation

hc-github-team-consul-core
Copy link
Collaborator

@hc-github-team-consul-core hc-github-team-consul-core commented Sep 29, 2023

Backport

This PR is auto-generated from #19031 to be assessed for backporting due to the inclusion of the label backport/1.16.

The below text is copied from the body of the original PR.


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

Overview of commits

JadhavPoonam and others added 30 commits August 11, 2023 15:52
* Initial protohcl implementation

Co-authored-by: Matt Keeler <[email protected]>
Co-authored-by: Daniel Upton <[email protected]>

* resourcehcl: implement resource decoding on top of protohcl

Co-authored-by: Daniel Upton <[email protected]>

* fix: resolve ci failures

* test: add additional unmarshalling tests

* refactor: update function test to clean protohcl package imports

---------

Co-authored-by: Matt Keeler <[email protected]>
Co-authored-by: Daniel Upton <[email protected]>
implement http read endpoint to expose resource grpc service read method
feat: list resources endpoint
Stop log producer before restarting container
Update BUSL LICENSE to include licensed product and version.

Co-authored-by: hashicorp-copywrite[bot] <110428419+hashicorp-copywrite[bot]@users.noreply.github.com>
resource: Make resource delete tenancy awarae
…ns BUSL header (#18485)

* Add license-checker action that fails when any backported file contains BUSL header

* Quote echoed variable to retain line breaks

* Add ticket to reference for more details
The `grep` command used to obtain the ID for the terminating gateway
role is not reliable in all scenarios. For example, if there is a
similarly named role, the command may return the wrong role ID for the
active terminating gateway instance.

This commit updates the command to use jq to obtain the role ID. If
multiple roles are found, jq will raise an error informing the user
that it cannot reliably determine the role ID.
* Update grep command to work on ubuntu

* Run license checker when new commits are pushed to PR
* Update README.md
* Update kv.mdx
* Add BUSL badge
* Support locality testing in consul-container

Support including locality in client sidecar config.

Also align test config structs with Ent to avoid future conflicts.

* Refactor consul-container fortio helpers

Refactor fortio test helpers to separate HTTP retries from waiting on
fortio result changes due to e.g. service startup and failovers.
* Fix HCL

* Update create-sameness-groups.mdx
…dpoints (#18500)

* NET-4853 - xds v2 - implement base connect proxy functionality for clusters

* NET-4853 - xds v2 - implement base connect proxy functionality for clusters

* NET-4932 - xds v2 - implement base connect proxy functionality for endpoints

* Update endpoints_test.go

* gofmt

* Update naming.go
* Support custom watches on controller
* refactor mapper methods
…utes (#18501)

* NET-4853 - xds v2 - implement base connect proxy functionality for clusters

* NET-4853 - xds v2 - implement base connect proxy functionality for clusters

* NET-4932 - xds v2 - implement base connect proxy functionality for endpoints

* Update endpoints_test.go

* gofmt

* NET-4858 - Make connect proxy route tests pass using xds v2

* Update endpoints_test.go

* Update naming.go

* use alsoRunTestForV2

* remove unused makeAddress

* gofmt

* fixing clusters
* CI Split integration tests to run nightly and every PR

* Checkout release branch for nightly test
* fix broken link caught in weekly report

* Apply suggestions from code review

Co-authored-by: Jeff Boruszak <[email protected]>

---------

Co-authored-by: Jeff Boruszak <[email protected]>
* Create nightly test-integration for consul release branch

* test

* fix
This commit fixes numerous spelling errors across the site and also
removes unnecessary whitespace that was present in the edited files.
* Make proto-public license MPL

* Add proto-public dir to exclusion list in .copywrite.hcl
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/cthain/fix-service-resolver-config-entry/instantly-chief-goblin branch from e5d3bb2 to ceb91b3 Compare September 29, 2023 17:40
@hc-github-team-consul-core hc-github-team-consul-core removed the request for review from a team September 29, 2023 17:40
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/cthain/fix-service-resolver-config-entry/instantly-chief-goblin branch from d664295 to d93dac8 Compare September 29, 2023 17:40
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto approved Consul Bot automated PR

@github-actions github-actions bot added type/docs Documentation needs to be created/updated/clarified theme/api Relating to the HTTP API interface theme/health-checks Health Check functionality theme/acls ACL and token generation theme/cli Flags and documentation for the CLI interface theme/config Relating to Consul Agent configuration, including reloading theme/ui Anything related to the UI theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies theme/telemetry Anything related to telemetry or observability type/ci Relating to continuous integration (CI) tooling for testing or releases pr/dependencies PR specifically updates dependencies of project theme/envoy/xds Related to Envoy support theme/contributing Additions and enhancements to community contributing materials theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics theme/consul-terraform-sync Relating to Consul Terraform Sync and Network Infrastructure Automation labels Sep 29, 2023
@vercel vercel bot temporarily deployed to Preview – consul September 29, 2023 17:46 Inactive
@hc-github-team-consul-core hc-github-team-consul-core merged commit 30c1a4d into release/1.16.x Sep 29, 2023
84 of 85 checks passed
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/cthain/fix-service-resolver-config-entry/instantly-chief-goblin branch September 29, 2023 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/dependencies PR specifically updates dependencies of project theme/acls ACL and token generation theme/api Relating to the HTTP API interface theme/cli Flags and documentation for the CLI interface theme/config Relating to Consul Agent configuration, including reloading theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies theme/consul-terraform-sync Relating to Consul Terraform Sync and Network Infrastructure Automation theme/contributing Additions and enhancements to community contributing materials theme/envoy/xds Related to Envoy support theme/health-checks Health Check functionality theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics theme/telemetry Anything related to telemetry or observability theme/ui Anything related to the UI type/ci Relating to continuous integration (CI) tooling for testing or releases type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.