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.

Sign up for GitHub

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

Jump to bottom

Fix a panic in connect-inject when the provided upstreams list is malformed #3956

Merged
merged 6 commits into from
May 6, 2024
Merged

Fix a panic in connect-inject when the provided upstreams list is malformed #3956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2b30a0d
Check if an upstream is malformed, if so ignore it.
dhiaayachi Apr 29, 2024
03c1887
support multiple upstreams separator (<space>, <comma>) add tests
dhiaayachi Apr 29, 2024
4af5e68
add /n as a separator
dhiaayachi Apr 29, 2024
4c471dd
add changelog
dhiaayachi Apr 29, 2024
13c989f
added log when upstream is skipped
dhiaayachi Apr 30, 2024
8646647
Merge branch 'main' into fix-panic-upstreams-annotations
dhiaayachi May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/3956.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:bug
control-plane: fix a panic when an upstream annotation is malformed.
```

```release-note:enhancement
control-plane: support <space>, <comma> and <\n> as upstream separators.
Copy link

Choose a reason for hiding this comment

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

❤️

```
8 changes: 7 additions & 1 deletion control-plane/connect-inject/webhook/container_env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ func (w *MeshWebhook) containerEnvVars(pod corev1.Pod) []corev1.EnvVar {
}

var result []corev1.EnvVar
for _, raw := range strings.Split(raw, ",") {
for _, raw := range strings.FieldsFunc(raw, func(r rune) bool {
return r == ',' || r == ' ' || r == '\n' // Split either comma separated or space separated
}) {
parts := strings.SplitN(raw, ":", 3)
if len(parts) < 2 {
w.Log.Error(fmt.Errorf("ustream URL is malformed, skipping it: %s", raw), "malformed upstream")
continue
}
port, _ := common.PortValue(pod, strings.TrimSpace(parts[1]))
if port > 0 {
name := strings.TrimSpace(parts[0])
Expand Down
127 changes: 118 additions & 9 deletions control-plane/connect-inject/webhook/container_env_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,131 @@ func TestContainerEnvVars(t *testing.T) {
cases := []struct {
Name string
Upstream string
required []corev1.EnvVar
}{
{
"Upstream with datacenter",
"static-server:7890:dc1",
[]corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
},
},
{
"Upstream without datacenter",
"static-server:7890",
[]corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
},
},
{
"Multiple upstreams comma separated",
"static-server:7890, static-server2:7892",
[]corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
{
Name: "STATIC_SERVER2_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER2_CONNECT_SERVICE_PORT",
Value: "7892",
},
},
},
{
"Multiple upstreams comma separated",
"static-server:7890, static-server2:7892 static-server3:7893",
[]corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
{
Name: "STATIC_SERVER2_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER2_CONNECT_SERVICE_PORT",
Value: "7892",
},
{
Name: "STATIC_SERVER3_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER3_CONNECT_SERVICE_PORT",
Value: "7893",
},
},
},
{
"Multiple upstreams comma separated and carriage return",
`static-server:7890,
static-server2:7892
static-server3:7893`,
[]corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
{
Name: "STATIC_SERVER2_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER2_CONNECT_SERVICE_PORT",
Value: "7892",
},
{
Name: "STATIC_SERVER3_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER3_CONNECT_SERVICE_PORT",
Value: "7893",
},
},
},
{
"Multiple upstreams comma separated and carriage return malformed upstream",
`static-server7890,
static-server2:7892
static-server3:7893`,
[]corev1.EnvVar{
{
Name: "STATIC_SERVER2_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER2_CONNECT_SERVICE_PORT",
Value: "7892",
},
{
Name: "STATIC_SERVER3_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER3_CONNECT_SERVICE_PORT",
Value: "7893",
},
},
},
}

Expand All @@ -42,15 +159,7 @@ func TestContainerEnvVars(t *testing.T) {
},
})

require.ElementsMatch(envVars, []corev1.EnvVar{
{
Name: "STATIC_SERVER_CONNECT_SERVICE_HOST",
Value: "127.0.0.1",
}, {
Name: "STATIC_SERVER_CONNECT_SERVICE_PORT",
Value: "7890",
},
})
require.ElementsMatch(envVars, tt.required)
})
}
}
Loading