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

Bug 1854479: enforce permissions on Thanos querier endpoints #908

Merged
merged 2 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions assets/thanos-querier/deployment.yaml
Expand Up @@ -129,6 +129,7 @@ spec:
- --tls-private-key-file=/etc/tls/private/tls.key
- --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
- --logtostderr=true
- --allow-paths=/api/v1/query,/api/v1/query_range
image: quay.io/coreos/kube-rbac-proxy:v0.6.0
name: kube-rbac-proxy
ports:
Expand Down Expand Up @@ -163,6 +164,7 @@ spec:
- --tls-private-key-file=/etc/tls/private/tls.key
- --tls-cipher-suites=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
- --logtostderr=true
- --allow-paths=/api/v1/rules
image: quay.io/coreos/kube-rbac-proxy:v0.6.0
name: kube-rbac-proxy-rules
ports:
Expand Down
2 changes: 2 additions & 0 deletions jsonnet/thanos-querier.jsonnet
Expand Up @@ -384,6 +384,7 @@ local thanosQuerierRules =
'--tls-private-key-file=/etc/tls/private/tls.key',
'--tls-cipher-suites=' + std.join(',', $._config.tlsCipherSuites),
'--logtostderr=true',
'--allow-paths=/api/v1/query,/api/v1/query_range',
],
terminationMessagePolicy: 'FallbackToLogsOnError',
volumeMounts: [
Expand Down Expand Up @@ -436,6 +437,7 @@ local thanosQuerierRules =
'--tls-private-key-file=/etc/tls/private/tls.key',
'--tls-cipher-suites=' + std.join(',', $._config.tlsCipherSuites),
'--logtostderr=true',
'--allow-paths=/api/v1/rules',
],
terminationMessagePolicy: 'FallbackToLogsOnError',
volumeMounts: [
Expand Down
4 changes: 2 additions & 2 deletions pkg/manifests/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

66 changes: 65 additions & 1 deletion test/e2e/user_workload_monitoring_test.go
Expand Up @@ -724,6 +724,46 @@ func assertTenancyForMetrics(t *testing.T) {
t.Fatalf("failed to query Thanos querier: %v", err)
}
}

// Check that the account doesn't have to access the rules endpoint.
err = framework.Poll(5*time.Second, time.Minute, func() error {
// The tenancy port (9092) is only exposed in-cluster so we need to use
// port forwarding to access kube-rbac-proxy.
host, cleanUp, err := f.ForwardPort(t, "thanos-querier", 9092)
if err != nil {
t.Fatal(err)
}
defer cleanUp()

client := framework.NewPrometheusClient(
host,
token,
&framework.QueryParameterInjector{
Name: "namespace",
Value: userWorkloadTestNs,
},
)

resp, err := client.Do("GET", "/api/v1/rules", nil)
if err != nil {
return err
}
defer resp.Body.Close()

b, err := ioutil.ReadAll(resp.Body)
if err != nil {
return err
}

if resp.StatusCode/100 == 2 {
return fmt.Errorf("expected request to be rejected, but got status code %d (%s)", resp.StatusCode, framework.ClampMax(b))
}

return nil
})
if err != nil {
t.Fatalf("the account has access to the rules endpoint of Thanos querier: %v", err)
}
}

// assertTenancyForRules ensures that a tenant can access rules from her namespace (and only from this one).
Expand Down Expand Up @@ -770,7 +810,6 @@ func assertTenancyForRules(t *testing.T) {
},
)

t.Logf("Retrieving rules")
err = framework.Poll(5*time.Second, time.Minute, func() error {
resp, err := client.Do("GET", "/api/v1/rules", nil)
if err != nil {
Expand Down Expand Up @@ -856,6 +895,31 @@ func assertTenancyForRules(t *testing.T) {
if err != nil {
t.Fatalf("failed to query rules from Thanos querier: %v", err)
}

// Check that the account doesn't have to access the query endpoints.
for _, path := range []string{"/api/v1/range?query=up", "/api/v1/query_range?query=up&start=0&end=0&step=1s"} {
err = framework.Poll(5*time.Second, time.Minute, func() error {
resp, err := client.Do("GET", path, nil)
if err != nil {
return err
}
defer resp.Body.Close()

b, err := ioutil.ReadAll(resp.Body)
if err != nil {
return err
}

if resp.StatusCode/100 == 2 {
return fmt.Errorf("unexpected status code response, got %d (%s)", resp.StatusCode, framework.ClampMax(b))
}

return nil
})
if err != nil {
t.Fatalf("the account has access to the %q endpoint of Thanos querier: %v", path, err)
}
}
}

func assertPrometheusAlertmanagerInUserNamespace(t *testing.T) {
Expand Down