Don't allow stat requests for named resources in --all-namespaces (#1163)

Don't allow the CLI or Web UI to request named resources if --all-namespaces is used.

This follows kubectl, which also does not allow requesting named resources
over all namespaces.

This PR also updates the Web API's behaviour to be in line with the CLI's. 
Both will now default to the default namespace if no namespace is specified.
This commit is contained in:
Risha Mars 2018-06-20 12:59:31 -07:00 committed by GitHub
parent 33ff1a33ab
commit 0ff1bb4ad8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 42 additions and 16 deletions

View File

@ -15,7 +15,6 @@ import (
"github.com/runconduit/conduit/pkg/k8s" "github.com/runconduit/conduit/pkg/k8s"
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"k8s.io/api/core/v1"
) )
type statOptions struct { type statOptions struct {
@ -309,14 +308,7 @@ func getNamePrefix(resourceType string) string {
} }
func buildStatSummaryRequest(resource []string, options *statOptions) (*pb.StatSummaryRequest, error) { func buildStatSummaryRequest(resource []string, options *statOptions) (*pb.StatSummaryRequest, error) {
targetNamespace := options.namespace target, err := util.BuildResource(options.namespace, resource...)
if options.allNamespaces {
targetNamespace = ""
} else if options.namespace == "" {
targetNamespace = v1.NamespaceDefault
}
target, err := util.BuildResource(targetNamespace, resource...)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -339,13 +331,14 @@ func buildStatSummaryRequest(resource []string, options *statOptions) (*pb.StatS
TimeWindow: options.timeWindow, TimeWindow: options.timeWindow,
ResourceName: target.Name, ResourceName: target.Name,
ResourceType: target.Type, ResourceType: target.Type,
Namespace: targetNamespace, Namespace: options.namespace,
ToName: toRes.Name, ToName: toRes.Name,
ToType: toRes.Type, ToType: toRes.Type,
ToNamespace: options.toNamespace, ToNamespace: options.toNamespace,
FromName: fromRes.Name, FromName: fromRes.Name,
FromType: fromRes.Type, FromType: fromRes.Type,
FromNamespace: options.fromNamespace, FromNamespace: options.fromNamespace,
AllNamespaces: options.allNamespaces,
} }
return util.BuildStatSummaryRequest(requestParams) return util.BuildStatSummaryRequest(requestParams)

View File

@ -34,4 +34,16 @@ emoji 1/2 100.00% 2.0rps 123ms 123ms 123ms
t.Fatalf("Wrong output:\n expected: \n%s\n, got: \n%s", expectedOutput, output) t.Fatalf("Wrong output:\n expected: \n%s\n, got: \n%s", expectedOutput, output)
} }
}) })
t.Run("Returns an error for named resource queries with the --all-namespaces flag", func(t *testing.T) {
options := newStatOptions()
options.allNamespaces = true
args := []string{"po", "web"}
expectedError := "stats for a resource cannot be retrieved by name across all namespaces"
_, err := buildStatSummaryRequest(args, options)
if err == nil || err.Error() != expectedError {
t.Fatalf("Expected error [%s] instead got [%s]", expectedError, err)
}
})
} }

View File

@ -9,6 +9,7 @@ import (
"github.com/runconduit/conduit/pkg/k8s" "github.com/runconduit/conduit/pkg/k8s"
"google.golang.org/grpc/codes" "google.golang.org/grpc/codes"
"google.golang.org/grpc/status" "google.golang.org/grpc/status"
"k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors" k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
@ -54,6 +55,7 @@ type StatSummaryRequestParams struct {
FromNamespace string FromNamespace string
FromType string FromType string
FromName string FromName string
AllNamespaces bool
} }
// GRPCError generates a gRPC error code, as defined in // GRPCError generates a gRPC error code, as defined in
@ -98,6 +100,17 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest
window = p.TimeWindow window = p.TimeWindow
} }
if p.AllNamespaces && p.ResourceName != "" {
return nil, errors.New("stats for a resource cannot be retrieved by name across all namespaces")
}
targetNamespace := p.Namespace
if p.AllNamespaces {
targetNamespace = ""
} else if p.Namespace == "" {
targetNamespace = v1.NamespaceDefault
}
resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType) resourceType, err := k8s.CanonicalKubernetesNameFromFriendlyName(p.ResourceType)
if err != nil { if err != nil {
return nil, err return nil, err
@ -106,7 +119,7 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest
statRequest := &pb.StatSummaryRequest{ statRequest := &pb.StatSummaryRequest{
Selector: &pb.ResourceSelection{ Selector: &pb.ResourceSelection{
Resource: &pb.Resource{ Resource: &pb.Resource{
Namespace: p.Namespace, Namespace: targetNamespace,
Name: p.ResourceName, Name: p.ResourceName,
Type: resourceType, Type: resourceType,
}, },
@ -116,7 +129,7 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest
if p.ToName != "" || p.ToType != "" || p.ToNamespace != "" { if p.ToName != "" || p.ToType != "" || p.ToNamespace != "" {
if p.ToNamespace == "" { if p.ToNamespace == "" {
p.ToNamespace = p.Namespace p.ToNamespace = targetNamespace
} }
if p.ToType == "" { if p.ToType == "" {
p.ToType = resourceType p.ToType = resourceType
@ -139,7 +152,7 @@ func BuildStatSummaryRequest(p StatSummaryRequestParams) (*pb.StatSummaryRequest
if p.FromName != "" || p.FromType != "" || p.FromNamespace != "" { if p.FromName != "" || p.FromType != "" || p.FromNamespace != "" {
if p.FromNamespace == "" { if p.FromNamespace == "" {
p.FromNamespace = p.Namespace p.FromNamespace = targetNamespace
} }
if p.FromType == "" { if p.FromType == "" {
p.FromType = resourceType p.FromType = resourceType

View File

@ -85,7 +85,7 @@ const ApiHelpers = (pathPrefix, defaultMetricsWindow = '1m') => {
} }
let baseUrl = '/api/tps-reports?resource_type=' + type; let baseUrl = '/api/tps-reports?resource_type=' + type;
return !namespace ? baseUrl : baseUrl + '&namespace=' + namespace; return !namespace ? baseUrl + '&all_namespaces=true' : baseUrl + '&namespace=' + namespace;
}; };
// maintain a list of a component's requests, // maintain a list of a component's requests,

View File

@ -131,6 +131,9 @@ export const processSingleResourceRollup = rawMetrics => {
if (_.size(result) > 1) { if (_.size(result) > 1) {
console.error("Multi metric returned; expected single metric."); console.error("Multi metric returned; expected single metric.");
} }
if (_.isEmpty(result)) {
return [];
}
return _.values(result)[0]; return _.values(result)[0];
}; };

View File

@ -285,13 +285,13 @@ describe('ApiHelpers', () => {
it('returns the correct rollup url for deployment overviews', () => { it('returns the correct rollup url for deployment overviews', () => {
api = ApiHelpers('/go/my/own/way'); api = ApiHelpers('/go/my/own/way');
let deploymentUrl = api.urlsForResource("deployment"); let deploymentUrl = api.urlsForResource("deployment");
expect(deploymentUrl).to.equal('/api/tps-reports?resource_type=deployment'); expect(deploymentUrl).to.equal('/api/tps-reports?resource_type=deployment&all_namespaces=true');
}); });
it('returns the correct rollup url for pod overviews', () => { it('returns the correct rollup url for pod overviews', () => {
api = ApiHelpers('/go/my/own/way'); api = ApiHelpers('/go/my/own/way');
let deploymentUrls = api.urlsForResource("pod"); let deploymentUrls = api.urlsForResource("pod");
expect(deploymentUrls).to.equal('/api/tps-reports?resource_type=pod'); expect(deploymentUrls).to.equal('/api/tps-reports?resource_type=pod&all_namespaces=true');
}); });
it('scopes the query to the provided namespace', () => { it('scopes the query to the provided namespace', () => {

View File

@ -70,6 +70,10 @@ func (h *handler) handleApiPods(w http.ResponseWriter, req *http.Request, p http
} }
func (h *handler) handleApiStat(w http.ResponseWriter, req *http.Request, p httprouter.Params) { func (h *handler) handleApiStat(w http.ResponseWriter, req *http.Request, p httprouter.Params) {
allNs := false
if req.FormValue("all_namespaces") == "true" {
allNs = true
}
requestParams := util.StatSummaryRequestParams{ requestParams := util.StatSummaryRequestParams{
TimeWindow: req.FormValue("window"), TimeWindow: req.FormValue("window"),
ResourceName: req.FormValue("resource_name"), ResourceName: req.FormValue("resource_name"),
@ -81,6 +85,7 @@ func (h *handler) handleApiStat(w http.ResponseWriter, req *http.Request, p http
FromName: req.FormValue("from_name"), FromName: req.FormValue("from_name"),
FromType: req.FormValue("from_type"), FromType: req.FormValue("from_type"),
FromNamespace: req.FormValue("from_namespace"), FromNamespace: req.FormValue("from_namespace"),
AllNamespaces: allNs,
} }
// default to returning deployment stats // default to returning deployment stats