Skip to content

Commit

Permalink
Support warnings for TLS secrets in VS
Browse files Browse the repository at this point in the history
  • Loading branch information
@pleshakov
pleshakov committed Dec 7, 2020
1 parent b64293d commit 2da1c32
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 43 deletions.
17 changes: 7 additions & 10 deletions internal/configs/virtualserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,16 +196,12 @@ type virtualServerConfigurator struct {
}

func (vsc *virtualServerConfigurator) addWarningf(obj runtime.Object, msgFmt string, args ...interface{}) {
vsc.addWarning(obj, fmt.Sprintf(msgFmt, args...))
}

func (vsc *virtualServerConfigurator) addWarning(obj runtime.Object, msg string) {
vsc.warnings[obj] = append(vsc.warnings[obj], msg)
vsc.warnings.AddWarningf(obj, msgFmt, args...)
}

func (vsc *virtualServerConfigurator) addWarnings(obj runtime.Object, msgs []string) {
for _, msg := range msgs {
vsc.addWarning(obj, msg)
vsc.warnings.AddWarning(obj, msg)
}
}

Expand Down Expand Up @@ -258,7 +254,7 @@ func (vsc *virtualServerConfigurator) generateEndpointsForUpstream(
func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualServerEx) (version2.VirtualServerConfig, Warnings) {
vsc.clearWarnings()

sslConfig := generateSSLConfig(vsEx.VirtualServer.Spec.TLS, vsEx.VirtualServer.Namespace, vsEx.SecretRefs, vsc.cfgParams)
sslConfig := vsc.generateSSLConfig(vsEx.VirtualServer, vsEx.VirtualServer.Spec.TLS, vsEx.VirtualServer.Namespace, vsEx.SecretRefs, vsc.cfgParams)
tlsRedirectConfig := generateTLSRedirectConfig(vsEx.VirtualServer.Spec.TLS)

policyOpts := policyOptions{
Expand Down Expand Up @@ -1780,7 +1776,8 @@ func getNameForSourceForMatchesRouteMapFromCondition(condition conf_v1.Condition
return condition.Variable
}

func generateSSLConfig(tls *conf_v1.TLS, namespace string, secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams) *version2.SSL {
func (vsc *virtualServerConfigurator) generateSSLConfig(owner runtime.Object, tls *conf_v1.TLS, namespace string,
secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams) *version2.SSL {
if tls == nil {
return nil
}
Expand All @@ -1797,11 +1794,11 @@ func generateSSLConfig(tls *conf_v1.TLS, namespace string, secretRefs map[string
if secret.Error != nil {
name = pemFileNameForMissingTLSSecret
ciphers = "NULL"
// TO-DO: add a warning
vsc.addWarningf(owner, "TLS secret %s is invalid: %v", tls.Secret, secret.Error)
} else if secret.Type != api_v1.SecretTypeTLS {
name = pemFileNameForMissingTLSSecret
ciphers = "NULL"
// TO-DO: add a warning
vsc.addWarningf(owner, "TLS secret %s is of a wrong type '%s', must be '%s'", tls.Secret, secret.Type, api_v1.SecretTypeTLS)
} else {
name = secret.Path
}
Expand Down
98 changes: 66 additions & 32 deletions internal/configs/virtualserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2157,7 +2157,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
"Policy default/allow-policy is missing or invalid",
},
Expand Down Expand Up @@ -2194,7 +2194,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Allow: []string{"127.0.0.1"},
Deny: []string{"127.0.0.2"},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
"AccessControl policy (or policies) with deny rules is overridden by policy (or policies) with allow rules",
},
Expand Down Expand Up @@ -2264,7 +2264,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
},
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`RateLimit policy "default/rateLimit-policy2" with limit request option dryRun=true is overridden to dryRun=false by the first policy reference in this context`,
`RateLimit policy "default/rateLimit-policy2" with limit request option logLevel=info is overridden to logLevel=error by the first policy reference in this context`,
Expand Down Expand Up @@ -2307,7 +2307,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`JWT policy "default/jwt-policy" references an invalid Secret: secret is invalid`,
},
Expand Down Expand Up @@ -2369,7 +2369,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Realm: "test",
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`Multiple jwt policies in the same context is not valid. JWT policy "default/jwt-policy2" will be ignored`,
},
Expand Down Expand Up @@ -2410,7 +2410,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`IngressMTLS policy "default/ingress-mtls-policy" references an invalid Secret: secret is invalid`,
},
Expand Down Expand Up @@ -2465,7 +2465,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
VerifyDepth: 1,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`Multiple ingressMTLS policies are not allowed. IngressMTLS policy "default/ingress-mtls-policy2" will be ignored`,
},
Expand Down Expand Up @@ -2507,7 +2507,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`IngressMTLS policy is not allowed in the route context`,
},
Expand Down Expand Up @@ -2549,7 +2549,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`TLS configuration needed for IngressMTLS policy`,
},
Expand Down Expand Up @@ -2612,7 +2612,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
SSLName: "$proxy_host",
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`Multiple egressMTLS policies in the same context is not valid. EgressMTLS policy "default/egress-mtls-policy2" will be ignored`,
},
Expand Down Expand Up @@ -2654,7 +2654,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`EgressMTLS policy "default/egress-mtls-policy" references an invalid Secret: secret is invalid`,
},
Expand Down Expand Up @@ -2696,7 +2696,7 @@ func TestGeneratePoliciesFails(t *testing.T) {
Code: 500,
},
},
expectedWarnings: map[runtime.Object][]string{
expectedWarnings: Warnings{
nil: {
`EgressMTLS policy "default/egress-mtls-policy" references an invalid Secret: secret is invalid`,
},
Expand Down Expand Up @@ -3326,27 +3326,30 @@ func TestGenerateLocationForRedirect(t *testing.T) {

func TestGenerateSSLConfig(t *testing.T) {
tests := []struct {
inputTLS *conf_v1.TLS
inputSecretRefs map[string]*secrets.SecretReference
inputCfgParams *ConfigParams
expected *version2.SSL
msg string
inputTLS *conf_v1.TLS
inputSecretRefs map[string]*secrets.SecretReference
inputCfgParams *ConfigParams
expectedSSL *version2.SSL
expectedWarnings Warnings
msg string
}{
{
inputTLS: nil,
inputSecretRefs: map[string]*secrets.SecretReference{},
inputCfgParams: &ConfigParams{},
expected: nil,
msg: "no TLS field",
inputTLS: nil,
inputSecretRefs: map[string]*secrets.SecretReference{},
inputCfgParams: &ConfigParams{},
expectedSSL: nil,
expectedWarnings: Warnings{},
msg: "no TLS field",
},
{
inputTLS: &conf_v1.TLS{
Secret: "",
},
inputSecretRefs: map[string]*secrets.SecretReference{},
inputCfgParams: &ConfigParams{},
expected: nil,
msg: "TLS field with empty secret",
inputSecretRefs: map[string]*secrets.SecretReference{},
inputCfgParams: &ConfigParams{},
expectedSSL: nil,
expectedWarnings: Warnings{},
msg: "TLS field with empty secret",
},
{
inputTLS: &conf_v1.TLS{
Expand All @@ -3358,14 +3361,38 @@ func TestGenerateSSLConfig(t *testing.T) {
Error: errors.New("secret doesn't exist"),
},
},
expected: &version2.SSL{
expectedSSL: &version2.SSL{
HTTP2: false,
Certificate: pemFileNameForMissingTLSSecret,
CertificateKey: pemFileNameForMissingTLSSecret,
Ciphers: "NULL",
},
expectedWarnings: Warnings{
nil: []string{"TLS secret secret is invalid: secret doesn't exist"},
},
msg: "secret doesn't exist in the cluster with HTTPS",
},
{
inputTLS: &conf_v1.TLS{
Secret: "secret",
},
inputCfgParams: &ConfigParams{},
inputSecretRefs: map[string]*secrets.SecretReference{
"default/secret": {
Type: secrets.SecretTypeCA,
},
},
expectedSSL: &version2.SSL{
HTTP2: false,
Certificate: pemFileNameForMissingTLSSecret,
CertificateKey: pemFileNameForMissingTLSSecret,
Ciphers: "NULL",
},
expectedWarnings: Warnings{
nil: []string{"TLS secret secret is of a wrong type 'nginx.org/ca', must be 'kubernetes.io/tls'"},
},
msg: "wrong secret type",
},
{
inputTLS: &conf_v1.TLS{
Secret: "secret",
Expand All @@ -3377,22 +3404,29 @@ func TestGenerateSSLConfig(t *testing.T) {
},
},
inputCfgParams: &ConfigParams{},
expected: &version2.SSL{
expectedSSL: &version2.SSL{
HTTP2: false,
Certificate: "secret.pem",
CertificateKey: "secret.pem",
Ciphers: "",
},
msg: "normal case with HTTPS",
expectedWarnings: Warnings{},
msg: "normal case with HTTPS",
},
}

namespace := "default"

for _, test := range tests {
result := generateSSLConfig(test.inputTLS, namespace, test.inputSecretRefs, test.inputCfgParams)
if !reflect.DeepEqual(result, test.expected) {
t.Errorf("generateSSLConfig() returned %v but expected %v for the case of %s", result, test.expected, test.msg)
vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{})

// it is ok to use nil as the owner
result := vsc.generateSSLConfig(nil, test.inputTLS, namespace, test.inputSecretRefs, test.inputCfgParams)
if !reflect.DeepEqual(result, test.expectedSSL) {
t.Errorf("generateSSLConfig() returned %v but expected %v for the case of %s", result, test.expectedSSL, test.msg)
}
if !reflect.DeepEqual(vsc.warnings, test.expectedWarnings) {
t.Errorf("generateSSLConfig() returned warnings of \n%v but expected \n%v for the case of %s", vsc.warnings, test.expectedWarnings, test.msg)
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion internal/configs/warnings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ func (w Warnings) Add(warnings Warnings) {
}
}

// Adds a warning for the specified object.
// Adds a warning for the specified object using the provided format and arguments.
func (w Warnings) AddWarningf(obj runtime.Object, msgFmt string, args ...interface{}) {
w[obj] = append(w[obj], fmt.Sprintf(msgFmt, args...))
}

// Adds a warning for the specified object.
func (w Warnings) AddWarning(obj runtime.Object, msg string) {
w[obj] = append(w[obj], msg)
}

0 comments on commit 2da1c32

Please sign in to comment.