Skip to content

Commit 1fb12f6

Browse files
Merge pull request #14819 from smarterclayton/validate_route
Add a diagnostic that runs extended validation on routes
2 parents 955efb1 + bd79ab1 commit 1fb12f6

File tree

2 files changed

+140
-19
lines changed

2 files changed

+140
-19
lines changed

pkg/cmd/admin/diagnostics/cluster.go

+31-19
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77

88
"k8s.io/apimachinery/pkg/util/sets"
9+
"k8s.io/client-go/rest"
910
clientcmd "k8s.io/client-go/tools/clientcmd"
1011
clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
1112
kclientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
@@ -30,6 +31,7 @@ var (
3031
clustdiags.MasterNodeName,
3132
clustdiags.MetricsApiProxyName,
3233
clustdiags.NodeDefinitionsName,
34+
clustdiags.RouteCertificateValidationName,
3335
clustdiags.ServiceExternalIPsName,
3436
)
3537
)
@@ -47,11 +49,14 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf
4749
kclusterClient kclientset.Interface
4850
)
4951

50-
clusterClient, kclusterClient, found, serverUrl, err := o.findClusterClients(rawConfig)
52+
config, clusterClient, kclusterClient, found, serverUrl, err := o.findClusterClients(rawConfig)
5153
if !found {
5254
o.Logger.Notice("CED1002", "Could not configure a client with cluster-admin permissions for the current server, so cluster diagnostics will be skipped")
5355
return nil, true, err
5456
}
57+
if err != nil {
58+
return nil, false, err
59+
}
5560

5661
diagnostics := []types.Diagnostic{}
5762
for _, diagnosticName := range requestedDiagnostics {
@@ -75,6 +80,8 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf
7580
d = &clustdiags.MetricsApiProxy{KubeClient: kclusterClient}
7681
case clustdiags.ServiceExternalIPsName:
7782
d = &clustdiags.ServiceExternalIPs{MasterConfigFile: o.MasterConfigLocation, KclusterClient: kclusterClient}
83+
case clustdiags.RouteCertificateValidationName:
84+
d = &clustdiags.RouteCertificateValidation{OsClient: clusterClient, RESTConfig: config}
7885
default:
7986
return nil, false, fmt.Errorf("unknown diagnostic: %v", diagnosticName)
8087
}
@@ -84,51 +91,56 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf
8491
}
8592

8693
// attempts to find which context in the config might be a cluster-admin for the server in the current context.
87-
func (o DiagnosticsOptions) findClusterClients(rawConfig *clientcmdapi.Config) (*client.Client, kclientset.Interface, bool, string, error) {
94+
func (o DiagnosticsOptions) findClusterClients(rawConfig *clientcmdapi.Config) (*rest.Config, *client.Client, kclientset.Interface, bool, string, error) {
8895
if o.ClientClusterContext != "" { // user has specified cluster context to use
8996
if context, exists := rawConfig.Contexts[o.ClientClusterContext]; exists {
9097
configErr := fmt.Errorf("Specified '%s' as cluster-admin context, but it was not found in your client configuration.", o.ClientClusterContext)
9198
o.Logger.Error("CED1003", configErr.Error())
92-
return nil, nil, false, "", configErr
93-
} else if os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, o.ClientClusterContext, context); found {
94-
return os, kube, true, serverUrl, err
99+
return nil, nil, nil, false, "", configErr
100+
} else if config, os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, o.ClientClusterContext, context); found {
101+
return config, os, kube, true, serverUrl, err
95102
} else {
96-
return nil, nil, false, "", err
103+
return nil, nil, nil, false, "", err
97104
}
98105
}
99106
currentContext, exists := rawConfig.Contexts[rawConfig.CurrentContext]
100107
if !exists { // config specified cluster admin context that doesn't exist; complain and quit
101108
configErr := fmt.Errorf("Current context '%s' not found in client configuration; will not attempt cluster diagnostics.", rawConfig.CurrentContext)
102109
o.Logger.Error("CED1004", configErr.Error())
103-
return nil, nil, false, "", configErr
110+
return nil, nil, nil, false, "", configErr
104111
}
105112
// check if current context is already cluster admin
106-
if os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, rawConfig.CurrentContext, currentContext); found {
107-
return os, kube, true, serverUrl, err
113+
if config, os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, rawConfig.CurrentContext, currentContext); found {
114+
return config, os, kube, true, serverUrl, err
108115
}
109116
// otherwise, for convenience, search for a context with the same server but with the system:admin user
110117
for name, context := range rawConfig.Contexts {
111118
if context.Cluster == currentContext.Cluster && name != rawConfig.CurrentContext && strings.HasPrefix(context.AuthInfo, "system:admin/") {
112-
if os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, name, context); found {
113-
return os, kube, true, serverUrl, err
119+
if config, os, kube, found, serverUrl, err := o.makeClusterClients(rawConfig, name, context); found {
120+
return config, os, kube, true, serverUrl, err
114121
} else {
115-
return nil, nil, false, "", err // don't try more than one such context, they'll probably fail the same
122+
return nil, nil, nil, false, "", err // don't try more than one such context, they'll probably fail the same
116123
}
117124
}
118125
}
119-
return nil, nil, false, "", nil
126+
return nil, nil, nil, false, "", nil
120127
}
121128

122129
// makes the client from the specified context and determines whether it is a cluster-admin.
123-
func (o DiagnosticsOptions) makeClusterClients(rawConfig *clientcmdapi.Config, contextName string, context *clientcmdapi.Context) (*client.Client, kclientset.Interface, bool, string, error) {
130+
func (o DiagnosticsOptions) makeClusterClients(rawConfig *clientcmdapi.Config, contextName string, context *clientcmdapi.Context) (*rest.Config, *client.Client, kclientset.Interface, bool, string, error) {
124131
overrides := &clientcmd.ConfigOverrides{Context: *context}
125132
clientConfig := clientcmd.NewDefaultClientConfig(*rawConfig, overrides)
126133
serverUrl := rawConfig.Clusters[context.Cluster].Server
127134
factory := osclientcmd.NewFactory(clientConfig)
135+
config, err := factory.ClientConfig()
136+
if err != nil {
137+
o.Logger.Debug("CED1006", fmt.Sprintf("Error creating client for context '%s':\n%v", contextName, err))
138+
return nil, nil, nil, false, "", nil
139+
}
128140
o.Logger.Debug("CED1005", fmt.Sprintf("Checking if context is cluster-admin: '%s'", contextName))
129141
if osClient, kubeClient, err := factory.Clients(); err != nil {
130142
o.Logger.Debug("CED1006", fmt.Sprintf("Error creating client for context '%s':\n%v", contextName, err))
131-
return nil, nil, false, "", nil
143+
return nil, nil, nil, false, "", nil
132144
} else {
133145
subjectAccessReview := authorizationapi.SubjectAccessReview{Action: authorizationapi.Action{
134146
// if you can do everything, you're the cluster admin.
@@ -139,16 +151,16 @@ func (o DiagnosticsOptions) makeClusterClients(rawConfig *clientcmdapi.Config, c
139151
if resp, err := osClient.SubjectAccessReviews().Create(&subjectAccessReview); err != nil {
140152
if regexp.MustCompile(`User "[\w:]+" cannot create \w+ at the cluster scope`).MatchString(err.Error()) {
141153
o.Logger.Debug("CED1007", fmt.Sprintf("Context '%s' does not have cluster-admin access:\n%v", contextName, err))
142-
return nil, nil, false, "", nil
154+
return nil, nil, nil, false, "", nil
143155
} else {
144156
o.Logger.Error("CED1008", fmt.Sprintf("Unknown error testing cluster-admin access for context '%s':\n%v", contextName, err))
145-
return nil, nil, false, "", err
157+
return nil, nil, nil, false, "", err
146158
}
147159
} else if resp.Allowed {
148160
o.Logger.Info("CED1009", fmt.Sprintf("Using context for cluster-admin access: '%s'", contextName))
149-
return osClient, kubeClient, true, serverUrl, nil
161+
return config, osClient, kubeClient, true, serverUrl, nil
150162
}
151163
}
152164
o.Logger.Debug("CED1010", fmt.Sprintf("Context does not have cluster-admin access: '%s'", contextName))
153-
return nil, nil, false, "", nil
165+
return nil, nil, nil, false, "", nil
154166
}
+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package cluster
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"strings"
7+
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/util/validation/field"
10+
"k8s.io/client-go/rest"
11+
kapi "k8s.io/kubernetes/pkg/api"
12+
13+
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
14+
"github.com/openshift/origin/pkg/client"
15+
"github.com/openshift/origin/pkg/diagnostics/types"
16+
routeapi "github.com/openshift/origin/pkg/route/apis/route"
17+
"github.com/openshift/origin/pkg/route/apis/route/validation"
18+
clientset "github.com/openshift/origin/pkg/route/generated/internalclientset"
19+
)
20+
21+
// RouteCertificateValidation is a Diagnostic to check that there is a working router.
22+
type RouteCertificateValidation struct {
23+
OsClient *client.Client
24+
RESTConfig *rest.Config
25+
}
26+
27+
const (
28+
RouteCertificateValidationName = "RouteCertificateValidation"
29+
30+
clGetRoutesFailed = `
31+
Client error while retrieving all routes. Client retrieved records
32+
before, so this is likely to be a transient error. Try running
33+
diagnostics again. If this message persists, there may be a permissions
34+
problem with getting records. The error was:
35+
36+
(%[1]T) %[1]v`
37+
)
38+
39+
func (d *RouteCertificateValidation) Name() string {
40+
return "RouteCertificateValidation"
41+
}
42+
43+
func (d *RouteCertificateValidation) Description() string {
44+
return "Check all route certificates for certificates that might be rejected by extended validation."
45+
}
46+
47+
func (d *RouteCertificateValidation) CanRun() (bool, error) {
48+
if d.RESTConfig == nil || d.OsClient == nil {
49+
return false, errors.New("must have OpenShift client configuration")
50+
}
51+
can, err := userCan(d.OsClient, authorizationapi.Action{
52+
Namespace: metav1.NamespaceAll,
53+
Verb: "get",
54+
Group: routeapi.GroupName,
55+
Resource: "routes",
56+
})
57+
if err != nil {
58+
return false, types.DiagnosticError{ID: "DRouCert2010", LogMessage: fmt.Sprintf(clientAccessError, err), Cause: err}
59+
} else if !can {
60+
return false, types.DiagnosticError{ID: "DRouCert2011", LogMessage: "Client does not have cluster-admin access", Cause: err}
61+
}
62+
return true, nil
63+
}
64+
65+
func (d *RouteCertificateValidation) Check() types.DiagnosticResult {
66+
r := types.NewDiagnosticResult(RouteCertificateValidationName)
67+
68+
client, err := clientset.NewForConfig(d.RESTConfig)
69+
if err != nil {
70+
r.Error("DRouCert2012", err, fmt.Sprintf(clientAccessError, err))
71+
return r
72+
}
73+
74+
routes, err := client.Route().Routes(metav1.NamespaceAll).List(metav1.ListOptions{})
75+
if err != nil {
76+
r.Error("DRouCert2013", err, fmt.Sprintf(clGetRoutesFailed, err))
77+
return r
78+
}
79+
80+
for _, route := range routes.Items {
81+
copied, err := kapi.Scheme.Copy(&route)
82+
if err != nil {
83+
r.Error("DRouCert2003", err, fmt.Errorf("unable to copy route: %v", err).Error())
84+
return r
85+
}
86+
original := copied.(*routeapi.Route)
87+
88+
errs := validation.ExtendedValidateRoute(&route)
89+
90+
if len(errs) == 0 {
91+
if !kapi.Semantic.DeepEqual(original, &route) {
92+
err := fmt.Errorf("Route was normalized when extended validation was run (route/%s -n %s).\nPlease verify that this route certificate contains no invalid data.\n", route.Name, route.Namespace)
93+
r.Warn("DRouCert2004", nil, err.Error())
94+
}
95+
continue
96+
}
97+
err = fmt.Errorf("Route failed extended validation (route/%s -n %s):\n%s", route.Name, route.Namespace, flattenErrors(errs))
98+
r.Error("DRouCert2005", nil, err.Error())
99+
}
100+
return r
101+
}
102+
103+
func flattenErrors(errs field.ErrorList) string {
104+
var out []string
105+
for i := range errs {
106+
out = append(out, fmt.Sprintf("* %s", errs[i].Error()))
107+
}
108+
return strings.Join(out, "\n")
109+
}

0 commit comments

Comments
 (0)