From bb5f84be6ba1ca2976566787e6c8892b444224b6 Mon Sep 17 00:00:00 2001 From: Nitya Dhanushkodi Date: Fri, 10 Jun 2022 16:23:31 -0700 Subject: [PATCH] Add Peer field to Intention CRD source intention (#1263) update source intentions with peer name and bump api module and change initiate -> establish --- .github/workflows/test.yml | 4 +- .../templates/crd-serviceintentions.yaml | 3 + .../api/v1alpha1/serviceintentions_types.go | 19 ++++-- .../v1alpha1/serviceintentions_types_test.go | 67 +++++++++++++++++++ ...onsul.hashicorp.com_serviceintentions.yaml | 3 + .../peering_dialer_controller.go | 14 ++-- .../peering_dialer_controller_test.go | 2 +- control-plane/go.mod | 2 +- control-plane/go.sum | 4 +- 9 files changed, 98 insertions(+), 20 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a193b30be..18e6a7e192 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -146,7 +146,7 @@ jobs: working-directory: control-plane run: | mkdir -p $HOME/bin - wget https://github.com/ndhanushkodi/binaries/releases/download/v2oss/consul -O consulbin && \ + wget https://github.com/ndhanushkodi/binaries/releases/download/v3oss/consul -O consulbin && \ mv consulbin $HOME/bin/consul && chmod +x $HOME/bin/consul @@ -194,7 +194,7 @@ jobs: working-directory: control-plane run: | mkdir -p $HOME/bin - wget https://github.com/ndhanushkodi/binaries/releases/download/v2ent/consul -O consulbin && \ + wget https://github.com/ndhanushkodi/binaries/releases/download/v3ent/consul -O consulbin && \ mv consulbin $HOME/bin/consul && chmod +x $HOME/bin/consul diff --git a/charts/consul/templates/crd-serviceintentions.yaml b/charts/consul/templates/crd-serviceintentions.yaml index 2d1ca29285..255dffc25b 100644 --- a/charts/consul/templates/crd-serviceintentions.yaml +++ b/charts/consul/templates/crd-serviceintentions.yaml @@ -101,6 +101,9 @@ spec: partition: description: Partition is the Admin Partition for the Name parameter. type: string + peer: + description: Peer is the peer name for the Name parameter. + type: string permissions: description: Permissions is the list of all additional L7 attributes that extend the intention match criteria. Permission precedence diff --git a/control-plane/api/v1alpha1/serviceintentions_types.go b/control-plane/api/v1alpha1/serviceintentions_types.go index 0bab3adc74..09528dda3c 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types.go +++ b/control-plane/api/v1alpha1/serviceintentions_types.go @@ -78,6 +78,8 @@ type SourceIntention struct { Name string `json:"name,omitempty"` // Namespace is the namespace for the Name parameter. Namespace string `json:"namespace,omitempty"` + // Peer is the peer name for the Name parameter. + Peer string `json:"peer,omitempty"` // Partition is the Admin Partition for the Name parameter. Partition string `json:"partition,omitempty"` // Action is required for an L4 intention, and should be set to one of @@ -270,7 +272,7 @@ func (in *ServiceIntentions) Validate(consulMeta common.ConsulMeta) error { } errs = append(errs, in.validateNamespaces(consulMeta.NamespacesEnabled)...) - errs = append(errs, in.validatePartitions(consulMeta.PartitionsEnabled)...) + errs = append(errs, in.validateSourcePeerAndPartitions(consulMeta.PartitionsEnabled)...) if len(errs) > 0 { return apierrors.NewInvalid( @@ -311,6 +313,7 @@ func (in *SourceIntention) toConsul() *capi.SourceIntention { Name: in.Name, Namespace: in.Namespace, Partition: in.Partition, + Peer: in.Peer, Action: in.Action.toConsul(), Permissions: in.Permissions.toConsul(), Description: in.Description, @@ -455,14 +458,16 @@ func (in *ServiceIntentions) validateNamespaces(namespacesEnabled bool) field.Er return errs } -func (in *ServiceIntentions) validatePartitions(partitionsEnabled bool) field.ErrorList { +func (in *ServiceIntentions) validateSourcePeerAndPartitions(partitionsEnabled bool) field.ErrorList { var errs field.ErrorList path := field.NewPath("spec") - if !partitionsEnabled { - for i, source := range in.Spec.Sources { - if source.Partition != "" { - errs = append(errs, field.Invalid(path.Child("sources").Index(i).Child("partition"), source.Partition, `Consul Enterprise Admin Partitions must be enabled to set source.partition`)) - } + for i, source := range in.Spec.Sources { + if source.Partition != "" && !partitionsEnabled { + errs = append(errs, field.Invalid(path.Child("sources").Index(i).Child("partition"), source.Partition, `Consul Enterprise Admin Partitions must be enabled to set source.partition`)) + } + + if source.Peer != "" && source.Partition != "" { + errs = append(errs, field.Invalid(path.Child("sources").Index(i), source, `Both source.peer and source.partition cannot be set.`)) } } return errs diff --git a/control-plane/api/v1alpha1/serviceintentions_types_test.go b/control-plane/api/v1alpha1/serviceintentions_types_test.go index e6fbb4109c..ee79fe8c99 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types_test.go +++ b/control-plane/api/v1alpha1/serviceintentions_types_test.go @@ -1311,6 +1311,73 @@ func TestServiceIntentions_Validate(t *testing.T) { `spec.sources[2].partition: Invalid value: "partition-foo": Consul Enterprise Admin Partitions must be enabled to set source.partition`, }, }, + "single source peer and partition specified": { + input: &ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "does-not-matter", + }, + Spec: ServiceIntentionsSpec{ + Destination: Destination{ + Name: "dest-service", + Namespace: "namespace-a", + }, + Sources: SourceIntentions{ + { + Name: "web", + Action: "allow", + Namespace: "namespace-b", + Partition: "partition-other", + Peer: "peer-other", + }, + { + Name: "db", + Action: "deny", + Namespace: "namespace-c", + }, + }, + }, + }, + namespacesEnabled: true, + partitionsEnabled: true, + expectedErrMsgs: []string{ + `spec.sources[0]: Invalid value: v1alpha1.SourceIntention{Name:"web", Namespace:"namespace-b", Peer:"peer-other", Partition:"partition-other", Action:"allow", Permissions:v1alpha1.IntentionPermissions(nil), Description:""}: Both source.peer and source.partition cannot be set.`, + }, + }, + "multiple source peer and partition specified": { + input: &ServiceIntentions{ + ObjectMeta: metav1.ObjectMeta{ + Name: "does-not-matter", + }, + Spec: ServiceIntentionsSpec{ + Destination: Destination{ + Name: "dest-service", + Namespace: "namespace-a", + }, + Sources: SourceIntentions{ + { + Name: "web", + Action: "allow", + Namespace: "namespace-b", + Partition: "partition-other", + Peer: "peer-other", + }, + { + Name: "db", + Action: "deny", + Namespace: "namespace-c", + Partition: "partition-2", + Peer: "peer-2", + }, + }, + }, + }, + namespacesEnabled: true, + partitionsEnabled: true, + expectedErrMsgs: []string{ + `spec.sources[0]: Invalid value: v1alpha1.SourceIntention{Name:"web", Namespace:"namespace-b", Peer:"peer-other", Partition:"partition-other", Action:"allow", Permissions:v1alpha1.IntentionPermissions(nil), Description:""}: Both source.peer and source.partition cannot be set.`, + `spec.sources[1]: Invalid value: v1alpha1.SourceIntention{Name:"db", Namespace:"namespace-c", Peer:"peer-2", Partition:"partition-2", Action:"deny", Permissions:v1alpha1.IntentionPermissions(nil), Description:""}: Both source.peer and source.partition cannot be set.`, + }, + }, } for name, testCase := range cases { t.Run(name, func(t *testing.T) { diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml index 3018796e4f..5a36e7e5d7 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml @@ -94,6 +94,9 @@ spec: partition: description: Partition is the Admin Partition for the Name parameter. type: string + peer: + description: Peer is the peer name for the Name parameter. + type: string permissions: description: Permissions is the list of all additional L7 attributes that extend the intention match criteria. Permission precedence diff --git a/control-plane/connect-inject/peering_dialer_controller.go b/control-plane/connect-inject/peering_dialer_controller.go index a93aa3b47b..3e4ee309e7 100644 --- a/control-plane/connect-inject/peering_dialer_controller.go +++ b/control-plane/connect-inject/peering_dialer_controller.go @@ -111,7 +111,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques // correct secret specified in the spec. r.Log.Info("the secret in status.secretRef doesn't exist or wasn't set, establishing peering with the existing spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace) peeringToken := specSecret.Data[dialer.Secret().Key] - if err := r.initiatePeering(ctx, dialer.Name, string(peeringToken)); err != nil { + if err := r.establishPeering(ctx, dialer.Name, string(peeringToken)); err != nil { r.updateStatusError(ctx, dialer, err) return ctrl.Result{}, err } else { @@ -134,7 +134,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques if peering == nil { r.Log.Info("status.secret exists, but the peering doesn't exist in Consul; establishing peering with the existing spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace) peeringToken := specSecret.Data[dialer.Secret().Key] - if err := r.initiatePeering(ctx, dialer.Name, string(peeringToken)); err != nil { + if err := r.establishPeering(ctx, dialer.Name, string(peeringToken)); err != nil { r.updateStatusError(ctx, dialer, err) return ctrl.Result{}, err } else { @@ -148,7 +148,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques if r.specStatusSecretsDifferent(dialer, specSecret) { r.Log.Info("the secret in status.secretRef exists and is different from spec.peer.secret; establishing peering with the existing spec.peer.secret", "secret-name", dialer.Secret().Name, "secret-namespace", dialer.Namespace) peeringToken := specSecret.Data[dialer.Secret().Key] - if err := r.initiatePeering(ctx, dialer.Name, string(peeringToken)); err != nil { + if err := r.establishPeering(ctx, dialer.Name, string(peeringToken)); err != nil { r.updateStatusError(ctx, dialer, err) return ctrl.Result{}, err } else { @@ -224,13 +224,13 @@ func (r *PeeringDialerController) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// initiatePeering is a helper function that calls the Consul api to generate a token for the peer. -func (r *PeeringDialerController) initiatePeering(ctx context.Context, peerName string, peeringToken string) error { - req := api.PeeringInitiateRequest{ +// establishPeering is a helper function that calls the Consul api to generate a token for the peer. +func (r *PeeringDialerController) establishPeering(ctx context.Context, peerName string, peeringToken string) error { + req := api.PeeringEstablishRequest{ PeerName: peerName, PeeringToken: peeringToken, } - _, _, err := r.ConsulClient.Peerings().Initiate(ctx, req, nil) + _, _, err := r.ConsulClient.Peerings().Establish(ctx, req, nil) if err != nil { r.Log.Error(err, "failed to initiate peering", "err", err) return err diff --git a/control-plane/connect-inject/peering_dialer_controller_test.go b/control-plane/connect-inject/peering_dialer_controller_test.go index 0a74638e3e..54257c90f1 100644 --- a/control-plane/connect-inject/peering_dialer_controller_test.go +++ b/control-plane/connect-inject/peering_dialer_controller_test.go @@ -277,7 +277,7 @@ func TestReconcileCreateUpdatePeeringDialer(t *testing.T) { require.NoError(t, err) if tt.peeringExists { - _, _, err := dialerClient.Peerings().Initiate(context.Background(), api.PeeringInitiateRequest{PeerName: tt.peeringName, PeeringToken: encodedPeeringToken}, nil) + _, _, err := dialerClient.Peerings().Establish(context.Background(), api.PeeringEstablishRequest{PeerName: tt.peeringName, PeeringToken: encodedPeeringToken}, nil) require.NoError(t, err) k8sObjects = append(k8sObjects, createSecret("dialer-token-old", "default", "token", "old-token")) } diff --git a/control-plane/go.mod b/control-plane/go.mod index 1a397ca392..db7b2d9ec7 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v0.4.0 github.com/google/go-cmp v0.5.7 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 - github.com/hashicorp/consul/api v1.10.1-0.20220525202017-d8d8c8603e9b + github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe github.com/hashicorp/consul/sdk v0.9.0 github.com/hashicorp/go-discover v0.0.0-20200812215701-c4b85f6ed31f github.com/hashicorp/go-hclog v0.16.1 diff --git a/control-plane/go.sum b/control-plane/go.sum index 1857f30905..8d26a90944 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -297,8 +297,8 @@ github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgf github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= -github.com/hashicorp/consul/api v1.10.1-0.20220525202017-d8d8c8603e9b h1:RQEq6tJP50/a68+Bylaljb9RaR5DeflzrqrXviG+W0k= -github.com/hashicorp/consul/api v1.10.1-0.20220525202017-d8d8c8603e9b/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ= +github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe h1:YQSxqFG8IsG/qCQaPLnimycM8bpU6UYVJ5fURrJmDS4= +github.com/hashicorp/consul/api v1.10.1-0.20220610161046-7001e1151cbe/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50 h1:GwbRRT+QxMRbYI608FGwTfcZ0iOVLX69B2ePjpQoyXw= github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50/go.mod h1:yPkX5Q6CsxTFMjQQDJwzeNmUUF5NUGGbrDsv9wTb8cw=