Custom paths not effective

[morningstart-liu]

Hi，
I tested paths functionality by referring to the profile of paths.md. But didn't get the result that I wanted, and then I got the runtime configuration (http://127.0.0.1:8484/trickster/config) and I didn't see the configuration that I had customized, so, can you see where the problem is, thanks you .

version: Trickster version: 2.0.0-beta2 (linux/amd64)

my config.yaml

request_rewriters:
  example_rewriter:
    instructions:
      - [ 'header', 'set', 'Req', 'www.test.com1111' ]
      - [ 'hostname', 'set', 'www.test.com' ]
      
frontend:
  listen_port: 8480
#  listen_address: ''
#  tls_listen_address: ''
#  tls_listen_port: 0
#  0 by default, unlimited.
#  connections_limit: 0

caches:
  default:
    provider: filesystem
    index:
#     reap_interval_ms defines how long the Cache Index reaper sleeps between reap cycles. Default is 3 (3s)
      reap_interval_ms: 3000
#     flush_interval_ms sets how often the Cache Index saves its metadata to the cache from application memory. Default is 5 (5s)
      flush_interval_ms: 5000
#     max_size_bytes indicates how large the cache can grow in bytes before the Index evicts least-recently-accessed items. default is 512MB
      max_size_bytes: 536870912
#     max_size_backoff_bytes indicates how far below max_size_bytes the cache size must be to complete a byte-size-based eviction exercise. default is 16MB
      max_size_backoff_bytes: 16777216
#     max_size_objects indicates how large the cache can grow in objects before the Index evicts least-recently-accessed items. default is 0 (infinite)
      max_size_objects: 0
#     max_size_backoff_objects indicates how far under max_size_objects the cache size must be to complete object-size-based eviction exercise. default is 100
      max_size_backoff_objects: 100
    filesystem:
#     cache_path defines the directory location under which the Trickster cache will be maintained
#     default is /tmp/trickster
      cache_path: /data/trickster
      
backends:
  default:
    req_rewriter_name: example_rewriter
    origin_url: http://www.test.com:8089
    provider: reverseproxy # use 'reverseproxy' for no caching  reverseproxycache
    path_routing_disabled: true
    cache_name: default
    is_default: true
    hosts: [ www.test.com ]
    max_object_size_bytes: 524288
    max_ttl_ms: 300000
    cache_key_prefix: test-prefix
    tracing_name: default
    healthcheck:
      verb: HEAD
      interval_ms: 10000
      path: /aaaa
      expected_codes: [ 200, 204, 206, 301, 302, 304, 404]
      failure_threshold: 2
      recovery_threshold: 2
    paths:
      server-status:
        path: /server-status/
        methods: [ GET, POST ]
        handler: proxycache
        match_type: prefix
        response_headers:
          Cache-Control: no-cache
          '-X-Server-IP': ''
          '-X-Server-Code': ''
          '-X-Cache-Detail': ''
          '-X-First-Read-Time': ''
          '-X-First-Read-Time': ''
          '-X-First-Connect-Time': ''
        request_headers:
          Host: www.test.com
      root:
        path: /xxx/
        methods: [ GET, POST, PUT ]
        handler: proxycache
        match_type: prefix 
        response_headers:
          Cache-Control: s-maxage=300
          '-Expires': ''
        request_headers:
          Host: www.test.com
          CDN: 1111111
      depot:
        path: /depot/
        methods: [ GET, POST, PUT ]
        handler: proxycache
        match_type: prefix 
        response_headers:
          Cache-Control: s-maxage=300
          '-Expires': ''
        request_headers:
          Host: www.test.com
          Connection: ''
          X-Forwarded-For: 127.0.0.1



    
metrics:
  listen_port: 8481   # available for scraping at http://<trickster>:<metrics.listen_port>/metrics

logging:
  log_level: debug
  log_file: ./trickster.log


tracing:
  default:
    provider: stdout
    service_name: trickster
    collector_url: http://jaeger:14268/api/traces
    sample_rate: 1.0
    tags:
      key1: "value1"
      key2: "value2"
    jaeger:
      endpoint_type: collector    
    stdout:
      pretty_print: false

metrics:
  listen_port: 8481
  listen_address: ''
 

runtime configuration just about paths :

    paths:
      /-1111111111:
        path: /
        match_type: prefix
        handler: proxy
        methods:
        - GET
        - HEAD
        - POST
        - PUT
        - DELETE
        - CONNECT
        - OPTIONS
        - TRACE
        - PATCH
        - PURGE
        no_metrics: false
        reqrewriter: []

[togear]

tihs issue maybe the same problem with issue655.

when I tested backends paths, i found paths is nil
I hope this is useful for the troubleshooting

(dlv) p c.Backends
map[string]*github.com/trickstercache/trickster/v2/pkg/backends/options.Options [
"default": *{
Hosts: []string len: 3, cap: 3, [
"www.test.com",
"dl.steam.clngaa.com",
"steam-ppio-origin.ngaa.top",
],
Provider: "reverseproxy",
OriginURL: "http://localhost:8080/",
TimeoutMS: 180000,
KeepAliveTimeoutMS: 300000,
MaxIdleConns: 20,
CacheName: "default",
CacheKeyPrefix: "test-prefix",
HealthCheck: ("github.com/trickstercache/trickster/v2/pkg/backends/healthcheck/options.Options")(0x9b8e230),
TimeseriesRetentionFactor: 1024,
TimeseriesEvictionMethodName: "oldest",
BackfillToleranceMS: 0,
BackfillTolerancePoints: 0,
Paths: map[string]*github.com/trickstercache/trickster/v2/pkg/proxy/paths/options.Options [],
NegativeCacheName: "default",
TimeseriesTTLMS: 21600000,
FastForwardTTLMS: 15000,
MaxTTLMS: 300000,
RevalidationFactor: 2,
MaxObjectSizeBytes: 524288,
CompressibleTypeList: []string len: 9, cap: 9, [
"text/html",
"text/javascript",
"text/css",
"text/plain",
"text/xml",
"text/json",
"application/json",
"application/javascript",
"application/xml",
],
TracingConfigName: "default",
RuleName: "",
ReqRewriterName: "",
MaxShardSizePoints: 0,
MaxShardSizeMS: 0,
ShardStepMS: 0,
ALBOptions: *github.com/trickstercache/trickster/v2/pkg/backends/alb/options.Options nil,
Prometheus: *github.com/trickstercache/trickster/v2/pkg/backends/prometheus/options.Options nil,
TLS: ("github.com/trickstercache/trickster/v2/pkg/proxy/tls/options.Options")(0x987ad80),
ForwardedHeaders: "standard",
IsDefault: true,
FastForwardDisable: false,
PathRoutingDisabled: true,
RequireTLS: false,
MultipartRangesDisabled: false,
DearticulateUpstreamRanges: false,
LatencyMinMS: 0,
LatencyMaxMS: 0,
Name: "default",
Router: github.com/trickstercache/trickster/v2/pkg/router.Router nil,
Timeout: 180000000000,
BackfillTolerance: 0,
ValueRetention: 0,
Scheme: "",
Host: "",
PathPrefix: "",
NegativeCache: github.com/trickstercache/trickster/v2/pkg/cache/negative.Lookup [],
TimeseriesRetention: 1024,
TimeseriesEvictionMethod: EvictionMethodOldest (0),
TimeseriesTTL: 21600000000000,
FastForwardTTL: net.defaultTCPKeepAlive (15000000000),
FastForwardPath: *github.com/trickstercache/trickster/v2/pkg/proxy/paths/options.Options nil,
MaxTTL: crypto/tls.ticketKeyRotation (86400000000000),
HTTPClient: *net/http.Client nil,
CompressibleTypes: map[string]interface {} nil,
RuleOptions: *github.com/trickstercache/trickster/v2/pkg/backends/rule/options.Options nil,
ReqRewriter: github.com/trickstercache/trickster/v2/pkg/proxy/request/rewriter.RewriteInstructions len: 0, cap: 0, nil,
DoesShard: false,
MaxShardSize: 0,
ShardStep: 0,
md: github.com/trickstercache/trickster/v2/pkg/util/yamlx.KeyLookup nil,},
]

[jnichols-git]

I thought we had fixed 655, but apparently not--I'll get on that later today. Seems like a plausible cause for the underlying issue @morningstart-liu is experiencing; I'll take a look at that afterwards and see if things are fixed.

[morningstart-liu]

but Custom lost. Custom in paths/options/options.go:SetDefaults()

[jnichols-git]

The configuration for paths works generally like this:

1. Enter bo.SetDefaults
2. Pass old options to po.SetDefaults
3. In that function, the old options are modified
4. Re-enter bo.SetDefaults
5. Clone modified options to new backend options
6. Return new backend options

Previously, step 3 (the bit in paths/options) was happening, but step 5 (in backends/options) wasn't. My testing shows custom paths coming through on the config HTTP request with this fix--if you're still seeing your issue, please let me know in more detail what's happening.

[togear]

We are testing a specific url (http://www.test.com/XXXX) will perform diffrent action according to the path configuration.

eg: for http://www.test.com/server-status/xxx that pefix with /server-status will add Header CDN: test1
http://www.test.com/depot/xxx that prefix with depot/ will add Header CDN: test2
others that prefix with / will add HTTP Header CDN: test3 when prxoy to origin
when I tested with the patch with path clone fix, I found the backend options contained the Path Variables, but po *po.Options seems to be nil, which caused UpdateHeader didn't work

[jnichols-git]

I see. Thank you for the clarification--I'll work on finding the cause for that today.

[togear]

+++ b/pkg/routing/routing.go
@@ -267,8 +267,8 @@ func RegisterPathRoutes(r router.Router, handlers map[string]http.Handler,
                                p2.Merge(p)
                                continue
                        }
-                       p3 := po.New()
-                       p3.Merge(p)
+                       p3 := o.Paths[k].Clone()
+                       //  p3.Merge(p)
                        pathsWithVerbs[k] = p3
                }
        }

I found the null pathConfig assignment， p3 always null even it's merged with p.
I am not sure if it was corrected, please help check the code

[jnichols-git]

Will do. I didn't find the time to run through causes today but this is a good lead for tomorrow, thank you.!

[morningstart-liu]

+++ b/pkg/proxy/paths/options/options.go
@@ -195,11 +195,12 @@ func (o *Options) Merge(o2 *Options) {
 var pathMembers = []string{"path", "match_type", "handler", "methods", "cache_key_params",
        "cache_key_headers", "default_ttl_ms", "request_headers", "response_headers",
        "response_headers", "response_code", "response_body", "no_metrics", "collapsed_forwarding",
-       "req_rewriter_name",
+       "req_rewriter_name", "request_params",
 }

it's lost"request_params" at this point ? if not , why ?

[jnichols-git]

@morningstart-liu Good question, not sure--I'm adding this to the PR for review.

@togear After some investigation I think there's a wider problem with rewriters outside of this issue; would it be alright if I made a new issue for investigating it? As far as I can tell the open PR solves the original problem of this issue, which is that custom path configuration was not being applied and returned through the API, but is now.

I'm not seeing rewriters being applied to paths, and I'm additionally not seeing the Custom field of paths reliably making it to the function you referenced. Merge relies on that field to know what needs copying, so when the Custom field is left empty, Merge does nothing. Need to discuss with other maintainers before moving forward on fixing that problem, as it's possible I'm missing some context here.

[togear]

@jakenichols2719 it will be all right to make other issues for Clarify the usage of Paths