From b58401480b2e07108f4ede043e79b43dc731244a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sun, 4 Oct 2020 13:54:56 -0400 Subject: [PATCH] http: Check HTTPUseCache in a single place HTTPUseCache is only used is a gate for allowing QueryOptions.UseCache to be enabled. By moving it to the place where the query options are set, this behaviour is more obvious. Also remove parseInternal which was an alias for parse. --- agent/catalog_endpoint.go | 9 +++++---- agent/discovery_chain_endpoint.go | 5 +++-- agent/http.go | 21 ++++++++------------- agent/prepared_query_endpoint.go | 2 +- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/agent/catalog_endpoint.go b/agent/catalog_endpoint.go index f906954b4..9b26f9f4d 100644 --- a/agent/catalog_endpoint.go +++ b/agent/catalog_endpoint.go @@ -5,8 +5,9 @@ import ( "net/http" "strings" - "github.com/armon/go-metrics" + metrics "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" + cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/structs" ) @@ -200,7 +201,7 @@ func (s *HTTPHandlers) CatalogDatacenters(resp http.ResponseWriter, req *http.Re parseCacheControl(resp, req, &args.QueryOptions) var out []string - if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { + if args.QueryOptions.UseCache { raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogDatacentersName, &args) if err != nil { metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_datacenters"}, 1, @@ -282,7 +283,7 @@ func (s *HTTPHandlers) CatalogServices(resp http.ResponseWriter, req *http.Reque var out structs.IndexedServices defer setMeta(resp, &out.QueryMeta) - if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { + if args.QueryOptions.UseCache { raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogListServicesName, &args) if err != nil { metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_services"}, 1, @@ -371,7 +372,7 @@ func (s *HTTPHandlers) catalogServiceNodes(resp http.ResponseWriter, req *http.R var out structs.IndexedServiceNodes defer setMeta(resp, &out.QueryMeta) - if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { + if args.QueryOptions.UseCache { raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CatalogServicesName, &args) if err != nil { metrics.IncrCounterWithLabels([]string{"client", "rpc", "error", "catalog_service_nodes"}, 1, diff --git a/agent/discovery_chain_endpoint.go b/agent/discovery_chain_endpoint.go index 2a3b3493e..454bfb06e 100644 --- a/agent/discovery_chain_endpoint.go +++ b/agent/discovery_chain_endpoint.go @@ -6,10 +6,11 @@ import ( "strings" "time" + "github.com/mitchellh/mapstructure" + cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/lib/decode" - "github.com/mitchellh/mapstructure" ) func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Request) (interface{}, error) { @@ -59,7 +60,7 @@ func (s *HTTPHandlers) DiscoveryChainRead(resp http.ResponseWriter, req *http.Re var out structs.DiscoveryChainResponse defer setMeta(resp, &out.QueryMeta) - if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { + if args.QueryOptions.UseCache { raw, m, err := s.agent.cache.Get(req.Context(), cachetype.CompiledDiscoveryChainName, &args) if err != nil { return nil, err diff --git a/agent/http.go b/agent/http.go index 93c6cdc8f..8f618f8eb 100644 --- a/agent/http.go +++ b/agent/http.go @@ -18,6 +18,10 @@ import ( "github.com/NYTimes/gziphandler" "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" + "github.com/hashicorp/go-cleanhttp" + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/cache" "github.com/hashicorp/consul/agent/config" @@ -27,9 +31,6 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/logging" - "github.com/hashicorp/go-cleanhttp" - "github.com/mitchellh/mapstructure" - "github.com/pkg/errors" ) var HTTPSummaries = []prometheus.SummaryDefinition{ @@ -858,7 +859,7 @@ func (s *HTTPHandlers) parseConsistency(resp http.ResponseWriter, req *http.Requ if _, ok := query["leader"]; ok { defaults = false } - if _, ok := query["cached"]; ok { + if _, ok := query["cached"]; ok && s.agent.config.HTTPUseCache { b.SetUseCache(true) defaults = false } @@ -1037,9 +1038,9 @@ func parseMetaPair(raw string) (string, string) { return pair[0], "" } -// parseInternal is a convenience method for endpoints that need -// to use both parseWait and parseDC. -func (s *HTTPHandlers) parseInternal(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool { +// parse is a convenience method for endpoints that need to use both parseWait +// and parseDC. +func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool { s.parseDC(req, dc) var token string s.parseTokenWithDefault(req, &token) @@ -1056,12 +1057,6 @@ func (s *HTTPHandlers) parseInternal(resp http.ResponseWriter, req *http.Request return parseWait(resp, req, b) } -// parse is a convenience method for endpoints that need -// to use both parseWait and parseDC. -func (s *HTTPHandlers) parse(resp http.ResponseWriter, req *http.Request, dc *string, b structs.QueryOptionsCompat) bool { - return s.parseInternal(resp, req, dc, b) -} - func (s *HTTPHandlers) checkWriteAccess(req *http.Request) error { if req.Method == http.MethodGet || req.Method == http.MethodHead || req.Method == http.MethodOptions { return nil diff --git a/agent/prepared_query_endpoint.go b/agent/prepared_query_endpoint.go index 6e793989b..887c13c6d 100644 --- a/agent/prepared_query_endpoint.go +++ b/agent/prepared_query_endpoint.go @@ -120,7 +120,7 @@ func (s *HTTPHandlers) preparedQueryExecute(id string, resp http.ResponseWriter, var reply structs.PreparedQueryExecuteResponse defer setMeta(resp, &reply.QueryMeta) - if s.agent.config.HTTPUseCache && args.QueryOptions.UseCache { + if args.QueryOptions.UseCache { raw, m, err := s.agent.cache.Get(req.Context(), cachetype.PreparedQueryName, &args) if err != nil { // Don't return error if StaleIfError is set and we are within it and had