weiguoqiang/cert-manager
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #6565 from ThatsMrTalbot/fix/flag-validation-not-called

Browse Source 
fix: validation functions are not called anywhere
This commit is contained in:
jetstack-bot 2023-12-21 09:11:11 +00:00 committed by GitHub
commit ebb955f3f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 444 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
cmd/cainjector/app/cainjector.go
View File

@ -27,6 +27,7 @@ import (
"github.com/cert-manager/cert-manager/cainjector-binary/app/options"
config "github.com/cert-manager/cert-manager/internal/apis/config/cainjector"
"github.com/cert-manager/cert-manager/internal/apis/config/cainjector/validation"
cmdutil "github.com/cert-manager/cert-manager/internal/cmd/util"
cainjectorconfigfile "github.com/cert-manager/cert-manager/pkg/cainjector/configfile"
@ -88,6 +89,10 @@ servers and webhook servers.`,
return err
}
if err := validation.ValidateCAInjectorConfiguration(cainjectorConfig); err != nil {
return fmt.Errorf("error validating flags: %w", err)
}
if err := logf.ValidateAndApplyAsField(&cainjectorConfig.Logging, field.NewPath("logging")); err != nil {
return fmt.Errorf("failed to validate cainjector logging flags: %w", err)
}

5
cmd/controller/app/start.go
View File

@ -27,6 +27,7 @@ import (
"github.com/cert-manager/cert-manager/controller-binary/app/options"
config "github.com/cert-manager/cert-manager/internal/apis/config/controller"
"github.com/cert-manager/cert-manager/internal/apis/config/controller/validation"
cmdutil "github.com/cert-manager/cert-manager/internal/cmd/util"
_ "github.com/cert-manager/cert-manager/pkg/controller/acmechallenges"
@ -99,6 +100,10 @@ to renew certificates at an appropriate time before expiry.`,
return err
}
if err := validation.ValidateControllerConfiguration(controllerConfig); err != nil {
return fmt.Errorf("error validating flags: %w", err)
}
if err := logf.ValidateAndApplyAsField(&controllerConfig.Logging, field.NewPath("logging")); err != nil {
return fmt.Errorf("failed to validate controller logging flags: %w", err)
}

5
cmd/webhook/app/webhook.go
View File

@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
config "github.com/cert-manager/cert-manager/internal/apis/config/webhook"
"github.com/cert-manager/cert-manager/internal/apis/config/webhook/validation"
cmdutil "github.com/cert-manager/cert-manager/internal/cmd/util"
cmwebhook "github.com/cert-manager/cert-manager/internal/webhook"
logf "github.com/cert-manager/cert-manager/pkg/logs"
@ -94,6 +95,10 @@ functionality for cert-manager.`,
return err
}
if err := validation.ValidateWebhookConfiguration(webhookConfig); err != nil {
return fmt.Errorf("error validating flags: %w", err)
}
if err := logf.ValidateAndApplyAsField(&webhookConfig.Logging, field.NewPath("logging")); err != nil {
return fmt.Errorf("failed to validate webhook logging flags: %w", err)
}

40
internal/apis/config/cainjector/validation/validation_test.go Normal file
View File

@ -0,0 +1,40 @@
/*
Copyright 2021 The cert-manager Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validation
import (
"testing"
config "github.com/cert-manager/cert-manager/internal/apis/config/cainjector"
)
func TestValidateCAInjectorConfiguration(t *testing.T) {
tests := []struct {
name string
config *config.CAInjectorConfiguration
wantErr bool
}{
// TODO: Add test cases once validation function padded out.
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := ValidateCAInjectorConfiguration(tt.config); (err != nil) != tt.wantErr {
t.Errorf("ValidateCAInjectorConfiguration() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

215
internal/apis/config/controller/validation/validation_test.go Normal file
View File

@ -0,0 +1,215 @@
/*
Copyright 2021 The cert-manager Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validation
import (
"testing"
config "github.com/cert-manager/cert-manager/internal/apis/config/controller"
)
func TestValidateControllerConfiguration(t *testing.T) {
tests := []struct {
name string
config *config.ControllerConfiguration
wantErr bool
}{
{
"with valid config",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
},
false,
},
{
"with missing issuer kind",
&config.ControllerConfiguration{
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
},
true,
},
{
"with invalid kube-api-burst config",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: -1, // Must be positive
KubernetesAPIQPS: 1,
},
true,
},
{
"with invalid kube-api-burst config",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1, // Must be greater than KubernetesAPIQPS
KubernetesAPIQPS: 2,
},
true,
},
{
"with invalid kube-api-qps config",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: -1, // Must be positive
},
true,
},
{
"with valid acme http solver nameservers",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
ACMEHTTP01Config: config.ACMEHTTP01Config{
SolverNameservers: []string{
"1.1.1.1:53",
"8.8.8.8:53",
},
},
},
false,
},
{
"with invalid acme http solver nameserver missing port",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
ACMEHTTP01Config: config.ACMEHTTP01Config{
SolverNameservers: []string{
"1.1.1.1:53",
"8.8.8.8",
},
},
},
true,
},
{
"with valid acme dns recursive nameservers",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
ACMEDNS01Config: config.ACMEDNS01Config{
RecursiveNameservers: []string{
"1.1.1.1:53",
"https://example.com",
},
},
},
false,
},
{
"with inalid acme dns recursive nameserver missing port",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
ACMEDNS01Config: config.ACMEDNS01Config{
RecursiveNameservers: []string{
"1.1.1.1",
"https://example.com",
},
},
},
true,
},
// TODO: Turns out url.ParseRequestURI allows a lot of bad URLs through,
// including empty urls. We should replace that and uncomment this test.
//
// {
// "with inalid acme dns recursive nameserver invalid url",
// &config.ControllerConfiguration{
// IngressShimConfig: config.IngressShimConfig{
// DefaultIssuerKind: "Issuer",
// },
// KubernetesAPIBurst: 1,
// KubernetesAPIQPS: 1,
// ACMEDNS01Config: config.ACMEDNS01Config{
// RecursiveNameservers: []string{
// "1.1.1.1:53",
// "https://",
// },
// },
// },
// true,
// },
{
"with valid controllers named",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
Controllers: []string{"issuers", "clusterissuers"},
},
false,
},
{
"with wildcard controllers named",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
Controllers: []string{"*"},
},
false,
},
{
"with invalid controllers named",
&config.ControllerConfiguration{
IngressShimConfig: config.IngressShimConfig{
DefaultIssuerKind: "Issuer",
},
KubernetesAPIBurst: 1,
KubernetesAPIQPS: 1,
Controllers: []string{"foo"},
},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := ValidateControllerConfiguration(tt.config); (err != nil) != tt.wantErr {
t.Errorf("ValidateControllerConfiguration() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

174
internal/apis/config/webhook/validation/validation_test.go Normal file
View File

@ -0,0 +1,174 @@
/*
Copyright 2021 The cert-manager Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package validation
import (
"testing"
config "github.com/cert-manager/cert-manager/internal/apis/config/webhook"
)
func TestValidateWebhookConfiguration(t *testing.T) {
tests := []struct {
name string
config *config.WebhookConfiguration
wantErr bool
}{
{
"with no tls config",
&config.WebhookConfiguration{},
false,
},
{
"with both filesystem and dynamic tls configured",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Filesystem: config.FilesystemServingConfig{
CertFile: "/test.crt",
KeyFile: "/test.key",
},
Dynamic: config.DynamicServingConfig{
SecretNamespace: "cert-manager",
SecretName: "test",
DNSNames: []string{"example.com"},
},
},
},
true,
},
{
"with valid filesystem tls config",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Filesystem: config.FilesystemServingConfig{
CertFile: "/test.crt",
KeyFile: "/test.key",
},
},
},
false,
},
{
"with valid tls config missing keyfile",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Filesystem: config.FilesystemServingConfig{
CertFile: "/test.crt",
},
},
},
true,
},
{
"with valid tls config missing certfile",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Filesystem: config.FilesystemServingConfig{
KeyFile: "/test.key",
},
},
},
true,
},
{
"with valid dynamic tls config",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Dynamic: config.DynamicServingConfig{
SecretNamespace: "cert-manager",
SecretName: "test",
DNSNames: []string{"example.com"},
},
},
},
false,
},
{
"with dynamic tls missing secret namespace",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Dynamic: config.DynamicServingConfig{
SecretName: "test",
DNSNames: []string{"example.com"},
},
},
},
true,
},
{
"with dynamic tls missing secret name",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Dynamic: config.DynamicServingConfig{
SecretNamespace: "cert-manager",
DNSNames: []string{"example.com"},
},
},
},
true,
},
{
"with dynamic tls missing dns names",
&config.WebhookConfiguration{
TLSConfig: config.TLSConfig{
Dynamic: config.DynamicServingConfig{
SecretName: "test",
SecretNamespace: "cert-manager",
DNSNames: nil,
},
},
},
true,
},
{
"with valid healthz port",
&config.WebhookConfiguration{
HealthzPort: 8080,
},
false,
},
{
"with invalid healthz port",
&config.WebhookConfiguration{
HealthzPort: 99999999,
},
true,
},
{
"with valid secure port",
&config.WebhookConfiguration{
SecurePort: 8080,
},
false,
},
{
"with invalid secure port",
&config.WebhookConfiguration{
SecurePort: 99999999,
},
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := ValidateWebhookConfiguration(tt.config); (err != nil) != tt.wantErr {
t.Errorf("ValidateWebhookConfiguration() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}