Skip to content

Commit 6851417

Browse files
authored
[1.8] GOLDPANDA (#55)
* Implement mesh config pathNormalization * Send a print out original path of request (#32152) Avoid golang processing the URL which limits our ability to tests non-standard clients. (cherry picked from commit 78136cd) * Update filter types
1 parent 4672527 commit 6851417

File tree

8 files changed

+271
-127
lines changed

8 files changed

+271
-127
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ require (
3535
github.com/d4l3k/messagediff v1.2.1 // indirect
3636
github.com/davecgh/go-spew v1.1.1
3737
github.com/docker/distribution v2.7.1+incompatible
38-
github.com/envoyproxy/go-control-plane v0.9.8-0.20201019204000-12785f608982
38+
github.com/envoyproxy/go-control-plane v0.9.8-0.20210420150603-831d6316d7b9
3939
github.com/evanphx/json-patch v4.9.0+incompatible
4040
github.com/evanphx/json-patch/v5 v5.1.0
4141
github.com/fatih/color v1.9.0
@@ -103,7 +103,7 @@ require (
103103
gopkg.in/yaml.v2 v2.3.0
104104
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
105105
helm.sh/helm/v3 v3.2.4
106-
istio.io/api v0.0.0-20210406215405-038ea2bb43a0
106+
istio.io/api v0.0.0-20210419172736-4acdfb7450da
107107
istio.io/client-go v0.0.0-20200908160912-f99162621a1a
108108
istio.io/gogo-genproto v0.0.0-20201112235858-7e611cb4d738
109109
istio.io/pkg v0.0.0-20201112235759-c861803834b2

go.sum

Lines changed: 4 additions & 122 deletions
Large diffs are not rendered by default.

istio.deps

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@
44
"name": "PROXY_REPO_SHA",
55
"repoName": "proxy",
66
"file": "",
7-
"lastStableSHA": "f53502c8e8fd2400f47a0b8dead0587ff4e1076f"
7+
"lastStableSHA": "1273b7402a69f96e05c1c8017f2ea88d72c67fce"
88
}
99
]

pilot/pkg/networking/core/v1alpha3/listener.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1688,7 +1688,23 @@ func buildHTTPConnectionManager(listenerOpts buildListenerOpts, httpOpts *httpLi
16881688
connectionManager.AccessLog = []*accesslog.AccessLog{}
16891689
connectionManager.HttpFilters = filters
16901690
connectionManager.StatPrefix = httpOpts.statPrefix
1691-
connectionManager.NormalizePath = proto.BoolTrue
1691+
1692+
// Setup normalization
1693+
connectionManager.PathWithEscapedSlashesAction = hcm.HttpConnectionManager_KEEP_UNCHANGED
1694+
switch listenerOpts.push.Mesh.GetPathNormalization().GetNormalization() {
1695+
case meshconfig.MeshConfig_ProxyPathNormalization_NONE:
1696+
connectionManager.NormalizePath = proto.BoolFalse
1697+
case meshconfig.MeshConfig_ProxyPathNormalization_BASE, meshconfig.MeshConfig_ProxyPathNormalization_DEFAULT:
1698+
connectionManager.NormalizePath = proto.BoolTrue
1699+
case meshconfig.MeshConfig_ProxyPathNormalization_MERGE_SLASHES:
1700+
connectionManager.NormalizePath = proto.BoolTrue
1701+
connectionManager.MergeSlashes = true
1702+
case meshconfig.MeshConfig_ProxyPathNormalization_DECODE_AND_MERGE_SLASHES:
1703+
connectionManager.NormalizePath = proto.BoolTrue
1704+
connectionManager.MergeSlashes = true
1705+
connectionManager.PathWithEscapedSlashesAction = hcm.HttpConnectionManager_UNESCAPE_AND_FORWARD
1706+
}
1707+
16921708
if httpOpts.useRemoteAddress {
16931709
connectionManager.UseRemoteAddress = proto.BoolTrue
16941710
} else {

pkg/config/xds/filter_types.gen.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/test/echo/server/endpoint/http.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,8 @@ func (h *httpHandler) addResponsePayload(r *http.Request, body *bytes.Buffer) {
294294
writeField(body, response.ServiceVersionField, h.Version)
295295
writeField(body, response.ServicePortField, port)
296296
writeField(body, response.HostField, r.Host)
297-
writeField(body, response.URLField, r.URL.String())
297+
// Use raw path, we don't want golang normalizing anything since we use this for testing purposes
298+
writeField(body, response.URLField, r.RequestURI)
298299
writeField(body, response.ClusterField, h.Cluster)
299300

300301
writeField(body, "Method", r.Method)

pkg/test/echo/server/forwarder/http.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ func (c *httpProtocol) makeRequest(ctx context.Context, req *request) (string, e
6565
if err != nil {
6666
return "", err
6767
}
68+
// Use raw path, we don't want golang normalizing anything since we use this for testing purposes
69+
httpReq.URL.Opaque = httpReq.URL.RawPath
6870

6971
// Set the per-request timeout.
7072
ctx, cancel := context.WithTimeout(ctx, req.Timeout)
Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,242 @@
1+
// +build integ
2+
// Copyright Istio Authors
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package security
17+
18+
import (
19+
"context"
20+
"crypto/md5"
21+
"encoding/hex"
22+
"fmt"
23+
"io"
24+
"sync"
25+
"testing"
26+
27+
"github.com/hashicorp/go-multierror"
28+
"istio.io/istio/pkg/test/framework/resource"
29+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/types"
31+
32+
meshconfig "istio.io/api/mesh/v1alpha1"
33+
"istio.io/istio/pkg/test/framework"
34+
"istio.io/istio/pkg/test/framework/components/echo"
35+
"istio.io/istio/pkg/test/scopes"
36+
"istio.io/istio/pkg/util/gogoprotomarshal"
37+
)
38+
39+
func TestNormalization(t *testing.T) {
40+
framework.NewTest(t).
41+
Features("traffic.routing").
42+
Run(func(t framework.TestContext) {
43+
type expect struct {
44+
in, out string
45+
}
46+
cases := []struct {
47+
ntype meshconfig.MeshConfig_ProxyPathNormalization_NormalizationType
48+
expectations []expect
49+
}{
50+
{
51+
meshconfig.MeshConfig_ProxyPathNormalization_NONE,
52+
[]expect{
53+
{"/", "/"},
54+
{"/app/", "/app/"},
55+
{"/app/../admin", "/app/../admin"},
56+
{"/app", "/app"},
57+
{"/app//", "/app//"},
58+
{"/app/%2f", "/app/%2f"},
59+
{"/app%2f/", "/app%2f/"},
60+
{"/xyz%30..//abc", "/xyz%30..//abc"},
61+
{"/app/%2E./admin", "/app/%2E./admin"},
62+
{`/app\admin`, `/app\admin`},
63+
{`/app/\/\/\admin`, `/app/\/\/\admin`},
64+
{`/%2Fapp%5cadmin%5Cabc`, `/%2Fapp%5cadmin%5Cabc`},
65+
{`/%5Capp%2f%5c%2F%2e%2e%2fadmin%5c\abc`, `/%5Capp%2f%5c%2F%2e%2e%2fadmin%5c\abc`},
66+
{`/app//../admin`, `/app//../admin`},
67+
{`/app//../../admin`, `/app//../../admin`},
68+
},
69+
},
70+
{
71+
meshconfig.MeshConfig_ProxyPathNormalization_BASE,
72+
[]expect{
73+
{"/", "/"},
74+
{"/app/", "/app/"},
75+
{"/app/../admin", "/admin"},
76+
{"/app", "/app"},
77+
{"/app//", "/app//"},
78+
{"/app/%2f", "/app/%2f"},
79+
{"/app%2f/", "/app%2f/"},
80+
{"/xyz%30..//abc", "/xyz0..//abc"},
81+
{"/app/%2E./admin", "/admin"},
82+
{`/app\admin`, `/app/admin`},
83+
{`/app/\/\/\admin`, `/app//////admin`},
84+
{`/%2Fapp%5cadmin%5Cabc`, `/%2Fapp%5cadmin%5Cabc`},
85+
{`/%5Capp%2f%5c%2F%2e%2e%2fadmin%5c\abc`, `/%5Capp%2f%5c%2F..%2fadmin%5c/abc`},
86+
{`/app//../admin`, `/app/admin`},
87+
{`/app//../../admin`, `/admin`},
88+
},
89+
},
90+
{
91+
meshconfig.MeshConfig_ProxyPathNormalization_MERGE_SLASHES,
92+
[]expect{
93+
{"/", "/"},
94+
{"/app/", "/app/"},
95+
{"/app/../admin", "/admin"},
96+
{"/app", "/app"},
97+
{"/app//", "/app/"},
98+
{"/app/%2f", "/app/%2f"},
99+
{"/app%2f/", "/app%2f/"},
100+
{"/xyz%30..//abc", "/xyz0../abc"},
101+
{"/app/%2E./admin", "/admin"},
102+
{`/app\admin`, `/app/admin`},
103+
{`/app/\/\/\admin`, `/app/admin`},
104+
{`/%2Fapp%5cadmin%5Cabc`, `/%2Fapp%5cadmin%5Cabc`},
105+
{`/%5Capp%2f%5c%2F%2e%2e%2fadmin%5c\abc`, `/%5Capp%2f%5c%2F..%2fadmin%5c/abc`},
106+
{`/app//../admin`, `/app/admin`},
107+
{`/app//../../admin`, `/admin`},
108+
},
109+
},
110+
{
111+
meshconfig.MeshConfig_ProxyPathNormalization_DECODE_AND_MERGE_SLASHES,
112+
[]expect{
113+
{"/", "/"},
114+
{"/app/", "/app/"},
115+
{"/app/../admin", "/admin"},
116+
{"/app", "/app"},
117+
{"/app//", "/app/"},
118+
{"/app/%2f", "/app/"},
119+
{"/app%2f/", "/app/"},
120+
{"/xyz%30..//abc", "/xyz0../abc"},
121+
{"/app/%2E./admin", "/admin"},
122+
{`/app\admin`, `/app/admin`},
123+
{`/app/\/\/\admin`, `/app/admin`},
124+
{`/%2Fapp%5cadmin%5Cabc`, `/app/admin/abc`},
125+
{`/%5Capp%2f%5c%2F%2e%2e%2fadmin%5c\abc`, `/app/admin/abc`},
126+
{`/app//../admin`, `/app/admin`},
127+
{`/app//../../admin`, `/admin`},
128+
},
129+
},
130+
}
131+
for _, tt := range cases {
132+
t.NewSubTest(tt.ntype.String()).Run(func(t framework.TestContext) {
133+
PatchMeshConfig(t, ist.Settings().IstioNamespace, t.Clusters(), fmt.Sprintf(`
134+
pathNormalization:
135+
normalization: %v`, tt.ntype.String()))
136+
for _, c := range apps.A {
137+
for _, dst := range []echo.Instance{apps.B[0], apps.Naked[0]} {
138+
for _, tt := range tt.expectations {
139+
t.NewSubTest(fmt.Sprintf("%s/%s", dst.Config().Service, tt.in)).Run(func(t framework.TestContext) {
140+
c.CallWithRetryOrFail(t, echo.CallOptions{
141+
Target: dst,
142+
Path: tt.in,
143+
PortName: "http",
144+
Validator: echo.ExpectKey("URL", tt.out),
145+
})
146+
})
147+
}
148+
}
149+
}
150+
})
151+
}
152+
})
153+
}
154+
155+
func PatchMeshConfig(t framework.TestContext, ns string, clusters resource.Clusters, patch string) {
156+
errG := multierror.Group{}
157+
origCfg := map[string]string{}
158+
mu := sync.RWMutex{}
159+
160+
cmName := "istio"
161+
if rev := t.Settings().Revision; rev != "default" && rev != "" {
162+
cmName += "-" + rev
163+
}
164+
for _, c := range clusters {
165+
c := c
166+
errG.Go(func() error {
167+
cm, err := c.CoreV1().ConfigMaps(ns).Get(context.TODO(), cmName, v1.GetOptions{})
168+
if err != nil {
169+
return err
170+
}
171+
mcYaml, ok := cm.Data["mesh"]
172+
if !ok {
173+
return fmt.Errorf("mesh config was missing in istio config map for %s", c.Name())
174+
}
175+
mu.Lock()
176+
origCfg[c.Name()] = cm.Data["mesh"]
177+
mu.Unlock()
178+
mc := &meshconfig.MeshConfig{}
179+
if err := gogoprotomarshal.ApplyYAML(mcYaml, mc); err != nil {
180+
return err
181+
}
182+
if err := gogoprotomarshal.ApplyYAML(patch, mc); err != nil {
183+
return err
184+
}
185+
cm.Data["mesh"], err = gogoprotomarshal.ToYAML(mc)
186+
if err != nil {
187+
return err
188+
}
189+
_, err = c.CoreV1().ConfigMaps(ns).Update(context.TODO(), cm, v1.UpdateOptions{})
190+
if err != nil {
191+
return err
192+
}
193+
scopes.Framework.Infof("patched %s meshconfig:\n%s", c.Name(), cm.Data["mesh"])
194+
pl, err := c.CoreV1().Pods(ns).List(context.TODO(), v1.ListOptions{LabelSelector: "app=istiod"})
195+
if err != nil {
196+
return err
197+
}
198+
for _, pod := range pl.Items {
199+
patchBytes := fmt.Sprintf(`{ "metadata": {"annotations": { "test.istio.io/mesh-config-hash": "%s" } } }`, hash(cm.Data["mesh"]))
200+
// Trigger immediate kubelet resync, to avoid 1 min+ delay on update
201+
// https://github.com/kubernetes/kubernetes/issues/30189
202+
_, err := c.CoreV1().Pods(ns).Patch(context.TODO(), pod.Name,
203+
types.MergePatchType, []byte(patchBytes), v1.PatchOptions{FieldManager: "istio-ci"})
204+
if err != nil {
205+
return fmt.Errorf("patch %v: %v", patchBytes, err)
206+
}
207+
}
208+
return nil
209+
})
210+
}
211+
t.WhenDone(func() error {
212+
errG := multierror.Group{}
213+
mu.RLock()
214+
defer mu.RUnlock()
215+
for cn, mcYaml := range origCfg {
216+
cn, mcYaml := cn, mcYaml
217+
c := clusters.GetByName(cn)
218+
errG.Go(func() error {
219+
cm, err := c.CoreV1().ConfigMaps(ns).Get(context.TODO(), cmName, v1.GetOptions{})
220+
if err != nil {
221+
return err
222+
}
223+
cm.Data["mesh"] = mcYaml
224+
_, err = c.CoreV1().ConfigMaps(ns).Update(context.TODO(), cm, v1.UpdateOptions{})
225+
return err
226+
})
227+
}
228+
if err := errG.Wait().ErrorOrNil(); err != nil {
229+
return fmt.Errorf("failed cleaning up cluster-local config: %v", err)
230+
}
231+
return nil
232+
})
233+
if err := errG.Wait().ErrorOrNil(); err != nil {
234+
t.Fatal(err)
235+
}
236+
}
237+
238+
func hash(s string) string {
239+
h := md5.New()
240+
_, _ = io.WriteString(h, s)
241+
return hex.EncodeToString(h.Sum(nil))
242+
}

0 commit comments

Comments
 (0)