From 23b64d4af96ce997211adb353faefa1c78801779 Mon Sep 17 00:00:00 2001 From: Ashwin Venkatesh Date: Wed, 26 Aug 2020 14:13:05 -0400 Subject: [PATCH] Address PR comments --- api/v1alpha1/servicedefaults_types.go | 42 +- api/v1alpha1/servicedefaults_types_test.go | 686 ++++++++++++++---- api/v1alpha1/servicedefaults_webhook.go | 20 +- api/v1alpha1/types.go | 84 ++- commands.go | 2 +- .../consul.hashicorp.com_servicedefaults.yaml | 4 - config/prometheus/kustomization.yaml | 2 - config/prometheus/monitor.yaml | 16 - subcommand/controller/command.go | 59 +- 9 files changed, 666 insertions(+), 249 deletions(-) delete mode 100644 config/prometheus/kustomization.yaml delete mode 100644 config/prometheus/monitor.yaml diff --git a/api/v1alpha1/servicedefaults_types.go b/api/v1alpha1/servicedefaults_types.go index 9d6111032c..830198bb57 100644 --- a/api/v1alpha1/servicedefaults_types.go +++ b/api/v1alpha1/servicedefaults_types.go @@ -5,9 +5,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! -// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. - // ServiceDefaultsSpec defines the desired state of ServiceDefaults type ServiceDefaultsSpec struct { Protocol string `json:"protocol,omitempty"` @@ -47,50 +44,19 @@ func init() { SchemeBuilder.Register(&ServiceDefaults{}, &ServiceDefaultsList{}) } +// ToConsul converts the entry into it's Consul equivalent struct. func (s *ServiceDefaults) ToConsul() *consulapi.ServiceConfigEntry { return &consulapi.ServiceConfigEntry{ Kind: consulapi.ServiceDefaults, Name: s.Name, //Namespace: s.Namespace, // todo: don't set this unless enterprise - Protocol: s.Spec.Protocol, - MeshGateway: consulapi.MeshGatewayConfig{ - Mode: s.gatewayMode(), - }, - Expose: consulapi.ExposeConfig{ - Checks: s.Spec.Expose.Checks, - Paths: s.parseExposePath(), - }, + Protocol: s.Spec.Protocol, + MeshGateway: s.Spec.MeshGateway.ToConsul(), + Expose: s.Spec.Expose.ToConsul(), ExternalSNI: s.Spec.ExternalSNI, } } -func (s *ServiceDefaults) parseExposePath() []consulapi.ExposePath { - var paths []consulapi.ExposePath - for _, path := range s.Spec.Expose.Paths { - paths = append(paths, consulapi.ExposePath{ - ListenerPort: path.ListenerPort, - Path: path.Path, - LocalPathPort: path.LocalPathPort, - Protocol: path.Protocol, - ParsedFromCheck: path.ParsedFromCheck, - }) - } - return paths -} - -func (s *ServiceDefaults) gatewayMode() consulapi.MeshGatewayMode { - switch s.Spec.MeshGateway.Mode { - case "local": - return consulapi.MeshGatewayModeLocal - case "none": - return consulapi.MeshGatewayModeNone - case "remote": - return consulapi.MeshGatewayModeRemote - default: - return consulapi.MeshGatewayModeDefault - } -} - // MatchesConsul returns true if entry has the same config as this struct. func (s *ServiceDefaults) MatchesConsul(entry *consulapi.ServiceConfigEntry) bool { return s.Name == entry.GetName() && diff --git a/api/v1alpha1/servicedefaults_types_test.go b/api/v1alpha1/servicedefaults_types_test.go index c2f6f8e59d..3833e7dbce 100644 --- a/api/v1alpha1/servicedefaults_types_test.go +++ b/api/v1alpha1/servicedefaults_types_test.go @@ -8,9 +8,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func ToConsul(t *testing.T) { - cases := []ToConsulCase{ - { +func Test_ToConsul(t *testing.T) { + cases := map[string]struct { + input *ServiceDefaults + expected *capi.ServiceConfigEntry + }{ + "protocol:http,mode:remote": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -33,7 +36,7 @@ func ToConsul(t *testing.T) { }, }, }, - { + "protocol:https,mode:local,exposePaths:1,externalSNI": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -48,11 +51,10 @@ func ToConsul(t *testing.T) { Checks: true, Paths: []ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, @@ -71,44 +73,41 @@ func ToConsul(t *testing.T) { Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, ExternalSNI: "test-external-sni", }, }, - { + "protocol:\"\",mode:\"\",exposePaths:2,externalSNI": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", Namespace: "consul-configs", }, Spec: ServiceDefaultsSpec{ - Protocol: "https", + Protocol: "", MeshGateway: MeshGatewayConfig{ - Mode: "local", + Mode: "", }, Expose: ExposeConfig{ Checks: true, Paths: []ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, @@ -119,43 +118,115 @@ func ToConsul(t *testing.T) { Kind: capi.ServiceDefaults, Name: "my-test-service", Namespace: "", - Protocol: "https", + Protocol: "", MeshGateway: capi.MeshGatewayConfig{ - Mode: capi.MeshGatewayModeLocal, + Mode: capi.MeshGatewayModeDefault, }, Expose: capi.ExposeConfig{ Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, ExternalSNI: "test-external-sni", }, }, + "protocol:http,mode:none,exposePaths:1,exposeChecks:false": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "none", + }, + Expose: ExposeConfig{ + Checks: false, + Paths: []ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeNone, + }, + Expose: capi.ExposeConfig{ + Checks: false, + Paths: []capi.ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + }, + "protocol:https,mode:unsupported": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "unsupported", + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeDefault, + }, + }, + }, } - for _, testCase := range cases { - output := testCase.input.ToConsul() - require.Equal(t, testCase.expected, output) + for name, testCase := range cases { + t.Run(name, func(t *testing.T) { + output := testCase.input.ToConsul() + require.Equal(t, testCase.expected, output) + }) } } -func TestRun_MatchesConsul(t *testing.T) { - cases := []MatchesConsulCase{ - { +func Test_MatchesConsul(t *testing.T) { + cases := map[string]struct { + internal *ServiceDefaults + consul *capi.ServiceConfigEntry + matches bool + }{ + "name:matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -179,7 +250,31 @@ func TestRun_MatchesConsul(t *testing.T) { }, true, }, - { + "name:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "differently-named-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeRemote, + }, + }, + false, + }, + "protocol:matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -200,7 +295,128 @@ func TestRun_MatchesConsul(t *testing.T) { }, true, }, - { + "protocol:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeDefault, + }, + }, + false, + }, + "gatewayConfig:matches": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeRemote, + }, + }, + true, + }, + "gatewayConfig:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + }, + false, + }, + "externalSNI:matches": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + ExternalSNI: "test-external-sni", + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeRemote, + }, + ExternalSNI: "test-external-sni", + }, + true, + }, + "externalSNI:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "http", + MeshGateway: MeshGatewayConfig{ + Mode: "remote", + }, + ExternalSNI: "test-external-sni", + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "http", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + ExternalSNI: "different-external-sni", + }, + false, + }, + "expose.checks:matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -215,11 +431,10 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, @@ -238,11 +453,10 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, @@ -250,7 +464,55 @@ func TestRun_MatchesConsul(t *testing.T) { }, true, }, - { + "expose.checks:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-config", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + ExternalSNI: "test-external-sni", + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + Expose: capi.ExposeConfig{ + Checks: false, + Paths: []capi.ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + ExternalSNI: "test-external-sni", + }, + false, + }, + "expose.paths:matches": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -265,22 +527,19 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, - ExternalSNI: "test-external-sni", }, }, &capi.ServiceConfigEntry{ @@ -295,26 +554,23 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, - ExternalSNI: "test-external-sni", }, true, }, - { + "expose.paths.listenerPort:mismatched": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -329,18 +585,200 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []ExposePath{ { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, + }, + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + Expose: capi.ExposeConfig{ + Checks: true, + Paths: []capi.ExposePath{ + { + ListenerPort: 81, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + false, + }, + "expose.paths.path:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + Expose: capi.ExposeConfig{ + Checks: true, + Paths: []capi.ExposePath{ + { + ListenerPort: 80, + Path: "/differnt/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + false, + }, + "expose.paths.localPathPort:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + Expose: capi.ExposeConfig{ + Checks: true, + Paths: []capi.ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 21, + Protocol: "tcp", + }, + }, + }, + }, + false, + }, + "expose.paths.protocol:mismatched": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", + }, + }, + }, + }, + }, + &capi.ServiceConfigEntry{ + Kind: capi.ServiceDefaults, + Name: "my-test-service", + Namespace: "", + Protocol: "https", + MeshGateway: capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + }, + Expose: capi.ExposeConfig{ + Checks: true, + Paths: []capi.ExposePath{ + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "https", + }, + }, + }, + }, + false, + }, + "expose.paths:mismatched when path lengths are different": { + &ServiceDefaults{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-test-service", + Namespace: "consul-configs", + }, + Spec: ServiceDefaultsSpec{ + Protocol: "https", + MeshGateway: MeshGatewayConfig{ + Mode: "local", + }, + Expose: ExposeConfig{ + Checks: true, + Paths: []ExposePath{ + { + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", + }, + { + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, @@ -359,11 +797,10 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, @@ -371,7 +808,7 @@ func TestRun_MatchesConsul(t *testing.T) { }, false, }, - { + "expose.paths:match when paths orders are different": { &ServiceDefaults{ ObjectMeta: metav1.ObjectMeta{ Name: "my-test-service", @@ -386,18 +823,16 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []ExposePath{ { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, }, }, @@ -416,18 +851,16 @@ func TestRun_MatchesConsul(t *testing.T) { Checks: true, Paths: []capi.ExposePath{ { - ListenerPort: 80, - Path: "/test/path", - LocalPathPort: 42, - Protocol: "tcp", - ParsedFromCheck: true, + ListenerPort: 80, + Path: "/test/path", + LocalPathPort: 42, + Protocol: "tcp", }, { - ListenerPort: 8080, - Path: "/second/test/path", - LocalPathPort: 11, - Protocol: "https", - ParsedFromCheck: false, + ListenerPort: 8080, + Path: "/second/test/path", + LocalPathPort: 11, + Protocol: "https", }, }, }, @@ -437,19 +870,10 @@ func TestRun_MatchesConsul(t *testing.T) { }, } - for _, testCase := range cases { - result := testCase.internal.MatchesConsul(testCase.consul) - require.Equal(t, testCase.matches, result) + for name, testCase := range cases { + t.Run(name, func(t *testing.T) { + result := testCase.internal.MatchesConsul(testCase.consul) + require.Equal(t, testCase.matches, result) + }) } } - -type ToConsulCase struct { - input *ServiceDefaults - expected *capi.ServiceConfigEntry -} - -type MatchesConsulCase struct { - internal *ServiceDefaults - consul *capi.ServiceConfigEntry - matches bool -} diff --git a/api/v1alpha1/servicedefaults_webhook.go b/api/v1alpha1/servicedefaults_webhook.go index 61435cce73..97310cad28 100644 --- a/api/v1alpha1/servicedefaults_webhook.go +++ b/api/v1alpha1/servicedefaults_webhook.go @@ -5,48 +5,48 @@ import ( "fmt" "net/http" + "github.com/go-logr/logr" capi "github.com/hashicorp/consul/api" "k8s.io/api/admission/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// log is for logging in this package. -var servicedefaultslog = logf.Log.WithName("servicedefaults-resource") - -func NewServiceDefaultsValidator(client client.Client, consulClient *capi.Client) *serviceDefaultsValidator { +func NewServiceDefaultsValidator(client client.Client, consulClient *capi.Client, logger logr.Logger) *serviceDefaultsValidator { return &serviceDefaultsValidator{ Client: client, ConsulClient: consulClient, + Logger: logger, } } type serviceDefaultsValidator struct { client.Client ConsulClient *capi.Client + Logger logr.Logger decoder *admission.Decoder } +// Note: The path value in the below line is the path to the webhook. If it is updates, run code-gen, update subcommand/controller/command.go and the consul-helm value for the path to the webhook. // +kubebuilder:webhook:verbs=create;update,path=/mutate-v1alpha1-servicedefaults,mutating=true,failurePolicy=fail,groups=consul.hashicorp.com,resources=servicedefaults,versions=v1alpha1,name=mservicedefaults.consul.io func (v *serviceDefaultsValidator) Handle(ctx context.Context, req admission.Request) admission.Response { - svcDefaults := &ServiceDefaults{} - err := v.decoder.Decode(req, svcDefaults) + var svcDefaults ServiceDefaults + err := v.decoder.Decode(req, &svcDefaults) if err != nil { return admission.Errored(http.StatusBadRequest, err) } if req.Operation == v1beta1.Create { - servicedefaultslog.Info("validate create", "name", svcDefaults.Name) + v.Logger.Info("validate create", "name", svcDefaults.Name) var svcDefaultsList ServiceDefaultsList if err := v.Client.List(context.Background(), &svcDefaultsList); err != nil { return admission.Errored(http.StatusInternalServerError, err) } for _, item := range svcDefaultsList.Items { if item.Name == svcDefaults.Name { - return admission.Errored(http.StatusBadRequest, fmt.Errorf("ServiceDefaults resource with name %q is already defined in namespace %q – all ServiceDefaults resources must have unique names across namespaces", - svcDefaults.Name, item.Namespace)) + return admission.Errored(http.StatusBadRequest, fmt.Errorf("ServiceDefaults resource with name %q is already defined – all ServiceDefaults resources must have unique names across namespaces", + svcDefaults.Name)) } } } diff --git a/api/v1alpha1/types.go b/api/v1alpha1/types.go index 8428b87ad1..6581c883e5 100644 --- a/api/v1alpha1/types.go +++ b/api/v1alpha1/types.go @@ -1,7 +1,7 @@ package v1alpha1 import ( - "github.com/hashicorp/consul/api" + capi "github.com/hashicorp/consul/api" ) type MeshGatewayMode string @@ -32,6 +32,28 @@ type MeshGatewayConfig struct { Mode string `json:"mode,omitempty"` } +//ToConsul returns the MeshGatewayConfig for the entry +func (m MeshGatewayConfig) ToConsul() capi.MeshGatewayConfig { + switch m.Mode { + case string(capi.MeshGatewayModeLocal): + return capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeLocal, + } + case string(capi.MeshGatewayModeNone): + return capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeNone, + } + case string(capi.MeshGatewayModeRemote): + return capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeRemote, + } + default: + return capi.MeshGatewayConfig{ + Mode: capi.MeshGatewayModeDefault, + } + } +} + // ExposeConfig describes HTTP paths to expose through Envoy outside of Connect. // Users can expose individual paths and/or all HTTP/GRPC paths for checks. type ExposeConfig struct { @@ -43,20 +65,35 @@ type ExposeConfig struct { Paths []ExposePath `json:"paths,omitempty"` } -func (in *ExposeConfig) Matches(expose api.ExposeConfig) bool { - if in.Checks != expose.Checks { +type ExposePath struct { + // ListenerPort defines the port of the proxy's listener for exposed paths. + ListenerPort int `json:"listenerPort,omitempty"` + + // Path is the path to expose through the proxy, ie. "/metrics." + Path string `json:"path,omitempty"` + + // LocalPathPort is the port that the service is listening on for the given path. + LocalPathPort int `json:"localPathPort,omitempty"` + + // Protocol describes the upstream's service protocol. + // Valid values are "http" and "http2", defaults to "http" + Protocol string `json:"protocol,omitempty"` +} + +//Matches returns true if the expose config of the entry is the same as the struct +func (e ExposeConfig) Matches(expose capi.ExposeConfig) bool { + if e.Checks != expose.Checks { return false } - if len(in.Paths) != len(expose.Paths) { + if len(e.Paths) != len(expose.Paths) { return false } - for _, path := range in.Paths { + for _, path := range e.Paths { found := false for _, entryPath := range expose.Paths { - if path.ParsedFromCheck == entryPath.ParsedFromCheck && - path.Protocol == entryPath.Protocol && + if path.Protocol == entryPath.Protocol && path.Path == entryPath.Path && path.ListenerPort == entryPath.ListenerPort && path.LocalPathPort == entryPath.LocalPathPort { @@ -72,20 +109,23 @@ func (in *ExposeConfig) Matches(expose api.ExposeConfig) bool { return true } -type ExposePath struct { - // ListenerPort defines the port of the proxy's listener for exposed paths. - ListenerPort int `json:"listenerPort,omitempty"` - - // Path is the path to expose through the proxy, ie. "/metrics." - Path string `json:"path,omitempty"` - - // LocalPathPort is the port that the service is listening on for the given path. - LocalPathPort int `json:"localPathPort,omitempty"` - - // Protocol describes the upstream's service protocol. - // Valid values are "http" and "http2", defaults to "http" - Protocol string `json:"protocol,omitempty"` +//ToConsul returns the ExposeConfig for the entry +func (e ExposeConfig) ToConsul() capi.ExposeConfig { + return capi.ExposeConfig{ + Checks: e.Checks, + Paths: e.parseExposePath(), + } +} - // ParsedFromCheck is set if this path was parsed from a registered check - ParsedFromCheck bool `json:"parsedFromCheck,omitempty"` +func (e ExposeConfig) parseExposePath() []capi.ExposePath { + var paths []capi.ExposePath + for _, path := range e.Paths { + paths = append(paths, capi.ExposePath{ + ListenerPort: path.ListenerPort, + Path: path.Path, + LocalPathPort: path.LocalPathPort, + Protocol: path.Protocol, + }) + } + return paths } diff --git a/commands.go b/commands.go index c01d9cc138..8317e3e9e4 100644 --- a/commands.go +++ b/commands.go @@ -66,7 +66,7 @@ func init() { }, "controller": func() (cli.Command, error) { - return &cmdController.Command{}, nil + return &cmdController.Command{UI: ui}, nil }, } } diff --git a/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml b/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml index 4d745010e3..c1cbb35564 100644 --- a/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml +++ b/config/crd/bases/consul.hashicorp.com_servicedefaults.yaml @@ -63,10 +63,6 @@ spec: description: LocalPathPort is the port that the service is listening on for the given path. type: integer - parsedFromCheck: - description: ParsedFromCheck is set if this path was parsed - from a registered check - type: boolean path: description: Path is the path to expose through the proxy, ie. "/metrics." diff --git a/config/prometheus/kustomization.yaml b/config/prometheus/kustomization.yaml deleted file mode 100644 index ed137168a1..0000000000 --- a/config/prometheus/kustomization.yaml +++ /dev/null @@ -1,2 +0,0 @@ -resources: -- monitor.yaml diff --git a/config/prometheus/monitor.yaml b/config/prometheus/monitor.yaml deleted file mode 100644 index 9b8047b760..0000000000 --- a/config/prometheus/monitor.yaml +++ /dev/null @@ -1,16 +0,0 @@ - -# Prometheus Monitor Service (Metrics) -apiVersion: monitoring.coreos.com/v1 -kind: ServiceMonitor -metadata: - labels: - control-plane: controller-manager - name: controller-manager-metrics-monitor - namespace: system -spec: - endpoints: - - path: /metrics - port: https - selector: - matchLabels: - control-plane: controller-manager diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index 4298aa33ac..3031927f63 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -2,12 +2,14 @@ package controller import ( "flag" + "fmt" "os" "sync" "github.com/hashicorp/consul-k8s/api/v1alpha1" "github.com/hashicorp/consul-k8s/controllers" "github.com/hashicorp/consul-k8s/subcommand/flags" + "github.com/mitchellh/cli" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -17,25 +19,18 @@ import ( ) type Command struct { - flags *flag.FlagSet - k8s *flags.K8SFlags - httpFlags *flags.HTTPFlags - metricsAddr string - enableLeaderElection bool + UI cli.Ui + flagSet *flag.FlagSet + k8s *flags.K8SFlags + httpFlags *flags.HTTPFlags + + flagMetricsAddr string + flagEnableLeaderElection bool once sync.Once help string } -func (c *Command) Help() string { - c.once.Do(c.init) - return c.help -} - -func (c *Command) Synopsis() string { - return synopsis -} - var ( scheme = runtime.NewScheme() setupLog = ctrl.Log.WithName("setup") @@ -48,21 +43,24 @@ func init() { } func (c *Command) init() { - c.flags = flag.NewFlagSet("", flag.ContinueOnError) - c.flags.StringVar(&c.metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") - c.flags.BoolVar(&c.enableLeaderElection, "enable-leader-election", false, + c.flagSet = flag.NewFlagSet("", flag.ContinueOnError) + c.flagSet.StringVar(&c.flagMetricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") + c.flagSet.BoolVar(&c.flagEnableLeaderElection, "enable-leader-election", false, "Enable leader election for controller. "+ "Enabling this will ensure there is only one active controller manager.") c.httpFlags = &flags.HTTPFlags{} - flags.Merge(c.flags, c.httpFlags.Flags()) - c.help = flags.Usage(help, c.flags) + flags.Merge(c.flagSet, c.httpFlags.Flags()) + c.help = flags.Usage(help, c.flagSet) } func (c *Command) Run(_ []string) int { c.once.Do(c.init) - - if err := c.flags.Parse(nil); err != nil { - setupLog.Error(err, "parsing flags") + if err := c.flagSet.Parse(nil); err != nil { + setupLog.Error(err, "parsing flagSet") + return 1 + } + if len(c.flagSet.Args()) > 0 { + c.UI.Error(fmt.Sprintf("Should have no non-flag arguments.")) return 1 } @@ -70,9 +68,9 @@ func (c *Command) Run(_ []string) int { mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, - MetricsBindAddress: c.metricsAddr, + MetricsBindAddress: c.flagMetricsAddr, Port: 9443, - LeaderElection: c.enableLeaderElection, + LeaderElection: c.flagEnableLeaderElection, LeaderElectionID: "consul.hashicorp.com", }) if err != nil { @@ -97,7 +95,9 @@ func (c *Command) Run(_ []string) int { } if os.Getenv("ENABLE_WEBHOOKS") != "false" { - mgr.GetWebhookServer().Register("/mutate-v1alpha1-servicedefaults", &webhook.Admission{Handler: v1alpha1.NewServiceDefaultsValidator(mgr.GetClient(), consulClient)}) + //Note: The path here should be identical to the one on the kubebuilder annotation in file api/v1alpha1/servicedefaults_webhook.go + mgr.GetWebhookServer().Register("/mutate-v1alpha1-servicedefaults", + &webhook.Admission{Handler: v1alpha1.NewServiceDefaultsValidator(mgr.GetClient(), consulClient, ctrl.Log.WithName("webhooks").WithName("ServiceDefaults"))}) } // +kubebuilder:scaffold:builder @@ -109,6 +109,15 @@ func (c *Command) Run(_ []string) int { return 0 } +func (c *Command) Help() string { + c.once.Do(c.init) + return c.help +} + +func (c *Command) Synopsis() string { + return synopsis +} + const synopsis = "Starts the Consul Kubernetes controller" const help = ` Usage: consul-k8s controller [options]