Skip to content

Commit

Permalink
update api module, update exported services validation (#1248)
Browse files Browse the repository at this point in the history
  • Loading branch information
ndhanushkodi committed Jun 13, 2022
1 parent 614bb76 commit fa1936c
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 70 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v1oss/consul -O consulbin && \
wget https://github.com/ndhanushkodi/binaries/releases/download/v2oss/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul
Expand Down Expand Up @@ -194,7 +194,7 @@ jobs:
working-directory: control-plane
run: |
mkdir -p $HOME/bin
wget https://github.com/ndhanushkodi/binaries/releases/download/v1ent/consul -O consulbin && \
wget https://github.com/ndhanushkodi/binaries/releases/download/v2ent/consul -O consulbin && \
mv consulbin $HOME/bin/consul &&
chmod +x $HOME/bin/consul
Expand Down
25 changes: 13 additions & 12 deletions control-plane/api/v1alpha1/exportedservices_types.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package v1alpha1

import (
"errors"
"fmt"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -191,20 +190,16 @@ func (in *ExportedServices) MatchesConsul(candidate api.ConfigEntry) bool {

func (in *ExportedServices) Validate(consulMeta common.ConsulMeta) error {
var errs field.ErrorList
if !consulMeta.PartitionsEnabled {
return apierrors.NewForbidden(
schema.GroupResource{Group: ConsulHashicorpGroup, Resource: common.ExportedServices},
in.KubernetesName(),
errors.New("Consul Enterprise Admin Partitions must be enabled to create ExportedServices"))
}
if in.Name != consulMeta.Partition {
if consulMeta.PartitionsEnabled && in.Name != consulMeta.Partition {
errs = append(errs, field.Invalid(field.NewPath("name"), in.Name, fmt.Sprintf(`%s resource name must be the same name as the partition, "%s"`, in.KubeKind(), consulMeta.Partition)))
} else if !consulMeta.PartitionsEnabled && in.Name != "default" {
errs = append(errs, field.Invalid(field.NewPath("name"), in.Name, fmt.Sprintf(`%s resource name must be "default"`, in.KubeKind())))
}
if len(in.Spec.Services) == 0 {
errs = append(errs, field.Invalid(field.NewPath("spec").Child("services"), in.Spec.Services, "at least one service must be exported"))
}
for i, service := range in.Spec.Services {
if err := service.validate(field.NewPath("spec").Child("services").Index(i)); err != nil {
if err := service.validate(field.NewPath("spec").Child("services").Index(i), consulMeta); err != nil {
errs = append(errs, err...)
}
}
Expand All @@ -216,26 +211,32 @@ func (in *ExportedServices) Validate(consulMeta common.ConsulMeta) error {
return nil
}

func (in *ExportedService) validate(path *field.Path) field.ErrorList {
func (in *ExportedService) validate(path *field.Path, consulMeta common.ConsulMeta) field.ErrorList {
var errs field.ErrorList
if len(in.Consumers) == 0 {
errs = append(errs, field.Invalid(path, in.Consumers, "service must have at least 1 consumer."))
}
if !consulMeta.NamespacesEnabled && in.Namespace != "" {
errs = append(errs, field.Invalid(path, in.Namespace, "Consul Namespaces must be enabled to specify service namespace."))
}
for i, consumer := range in.Consumers {
if err := consumer.validate(path.Child("consumers").Index(i)); err != nil {
if err := consumer.validate(path.Child("consumers").Index(i), consulMeta); err != nil {
errs = append(errs, err)
}
}
return errs
}

func (in *ServiceConsumer) validate(path *field.Path) *field.Error {
func (in *ServiceConsumer) validate(path *field.Path, consulMeta common.ConsulMeta) *field.Error {
if in.Partition != "" && in.PeerName != "" {
return field.Invalid(path, *in, "both partition and peerName cannot be specified.")
}
if in.Partition == "" && in.PeerName == "" {
return field.Invalid(path, *in, "either partition or peerName must be specified.")
}
if !consulMeta.PartitionsEnabled && in.Partition != "" {
return field.Invalid(path.Child("partitions"), in.Partition, "Consul Admin Partitions need to be enabled to specify partition.")
}
return nil
}

Expand Down
8 changes: 5 additions & 3 deletions control-plane/api/v1alpha1/exportedservices_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestValidateExportedServices(t *testing.T) {
},
consulMeta: common.ConsulMeta{
PartitionsEnabled: true,
NamespacesEnabled: true,
Partition: otherPartition,
},
expAllow: true,
Expand Down Expand Up @@ -93,6 +94,7 @@ func TestValidateExportedServices(t *testing.T) {
},
consulMeta: common.ConsulMeta{
PartitionsEnabled: true,
NamespacesEnabled: true,
Partition: otherPartition,
},
expAllow: false,
Expand All @@ -102,13 +104,12 @@ func TestValidateExportedServices(t *testing.T) {
existingResources: []runtime.Object{},
newResource: &ExportedServices{
ObjectMeta: metav1.ObjectMeta{
Name: otherPartition,
Name: "default",
},
Spec: ExportedServicesSpec{
Services: []ExportedService{
{
Name: "service",
Namespace: "service-ns",
Consumers: []ServiceConsumer{{Partition: "other"}},
},
},
Expand All @@ -119,7 +120,7 @@ func TestValidateExportedServices(t *testing.T) {
Partition: "",
},
expAllow: false,
expErrMessage: "exportedservices.consul.hashicorp.com \"other\" is forbidden: Consul Enterprise Admin Partitions must be enabled to create ExportedServices",
expErrMessage: "exportedservices.consul.hashicorp.com \"default\" is invalid: spec.services[0].consumers[0].partitions: Invalid value: \"other\": Consul Admin Partitions need to be enabled to specify partition.",
},
"no services": {
existingResources: []runtime.Object{},
Expand Down Expand Up @@ -156,6 +157,7 @@ func TestValidateExportedServices(t *testing.T) {
},
consulMeta: common.ConsulMeta{
PartitionsEnabled: true,
NamespacesEnabled: true,
Partition: otherPartition,
},
expAllow: false,
Expand Down
28 changes: 13 additions & 15 deletions control-plane/connect-inject/peering_acceptor_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package connectinject
import (
"context"
"errors"
"net/http"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -54,7 +53,7 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ
// error), we need to delete it in Consul.
if k8serrors.IsNotFound(err) {
r.Log.Info("PeeringAcceptor was deleted, deleting from Consul", "name", req.Name, "ns", req.Namespace)
_, err := r.deletePeering(ctx, req.Name)
err := r.deletePeering(ctx, req.Name)
if err != nil {
return ctrl.Result{}, err
}
Expand All @@ -76,16 +75,18 @@ func (r *PeeringAcceptorController) Reconcile(ctx context.Context, req ctrl.Requ
}
}

// Read the peering from Consul.
readReq := api.PeeringReadRequest{Name: peeringAcceptor.Name}
peering, _, err := r.ConsulClient.Peerings().Read(ctx, readReq, nil)
var statusErr api.StatusError

var secretResourceVersion string

// Read the peering from Consul.
peering, _, err := r.ConsulClient.Peerings().Read(ctx, peeringAcceptor.Name, nil)
if err != nil {
r.Log.Error(err, "failed to get Peering from Consul", "name", req.Name)
return ctrl.Result{}, err
}

// If the peering doesn't exist in Consul, generate a new token, and store it in the specified backend. Store the
// current state in the status.
if errors.As(err, &statusErr) && statusErr.Code == http.StatusNotFound && peering == nil {
if peering == nil {
r.Log.Info("peering doesn't exist in Consul", "name", peeringAcceptor.Name)

if statusSecretSet {
Expand Down Expand Up @@ -307,16 +308,13 @@ func (r *PeeringAcceptorController) generateToken(ctx context.Context, peerName
}

// deletePeering is a helper function that calls the Consul api to delete a peering.
func (r *PeeringAcceptorController) deletePeering(ctx context.Context, peerName string) (*api.PeeringDeleteResponse, error) {
deleteReq := api.PeeringDeleteRequest{
Name: peerName,
}
resp, _, err := r.ConsulClient.Peerings().Delete(ctx, deleteReq, nil)
func (r *PeeringAcceptorController) deletePeering(ctx context.Context, peerName string) error {
_, err := r.ConsulClient.Peerings().Delete(ctx, peerName, nil)
if err != nil {
r.Log.Error(err, "failed to delete Peering from Consul", "name", peerName)
return nil, err
return err
}
return resp, nil
return nil
}

// createSecret is a helper function that creates a corev1.SecretRef when provided inputs.
Expand Down
11 changes: 3 additions & 8 deletions control-plane/connect-inject/peering_acceptor_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/base64"
"errors"
"net/http"
"testing"

logrtest "github.com/go-logr/logr/testing"
Expand Down Expand Up @@ -364,8 +363,7 @@ func TestReconcileCreateUpdatePeeringAcceptor(t *testing.T) {
require.False(t, resp.Requeue)

// After reconciliation, Consul should have the peering.
readReq := api.PeeringReadRequest{Name: "acceptor-created"}
peering, _, err := consulClient.Peerings().Read(context.Background(), readReq, nil)
peering, _, err := consulClient.Peerings().Read(context.Background(), "acceptor-created", nil)
require.NoError(t, err)
require.Equal(t, tt.expectedConsulPeerings[0].Name, peering.Name)
require.NotEmpty(t, peering.ID)
Expand Down Expand Up @@ -495,12 +493,9 @@ func TestReconcileDeletePeeringAcceptor(t *testing.T) {
require.False(t, resp.Requeue)

// After reconciliation, Consul should not have the peering.
readReq := api.PeeringReadRequest{Name: "acceptor-deleted"}
peering, _, err := consulClient.Peerings().Read(context.Background(), readReq, nil)
var statusErr api.StatusError
require.ErrorAs(t, err, &statusErr)
require.Equal(t, http.StatusNotFound, statusErr.Code)
peering, _, err := consulClient.Peerings().Read(context.Background(), "acceptor-deleted", nil)
require.Nil(t, peering)
require.NoError(t, err)
})
}
}
Expand Down
24 changes: 8 additions & 16 deletions control-plane/connect-inject/peering_dialer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package connectinject
import (
"context"
"errors"
"net/http"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -45,7 +44,7 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques
// error), we need to delete it in Consul.
if k8serrors.IsNotFound(err) {
r.Log.Info("PeeringDialer was deleted, deleting from Consul", "name", req.Name, "ns", req.Namespace)
_, err := r.deletePeering(ctx, req.Name)
err := r.deletePeering(ctx, req.Name)
if err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -111,17 +110,13 @@ func (r *PeeringDialerController) Reconcile(ctx context.Context, req ctrl.Reques

// Read the peering from Consul.
// TODO(peering): do we need to pass in partition?
readReq := api.PeeringReadRequest{Name: peeringDialer.Name}
r.Log.Info("reading peering from Consul", "name", peeringDialer.Name)
peering, _, err := r.ConsulClient.Peerings().Read(ctx, readReq, nil)
var statusErr api.StatusError
peeringExists := true
if errors.As(err, &statusErr) && statusErr.Code == http.StatusNotFound && peering == nil {
peeringExists = false
} else if err != nil {
peering, _, err := r.ConsulClient.Peerings().Read(ctx, peeringDialer.Name, nil)
if err != nil {
r.Log.Error(err, "failed to get Peering from Consul", "name", req.Name)
return ctrl.Result{}, err
}
peeringExists := peering != nil
// TODO(peering): Verify that the existing peering in Consul is an dialer peer. If it is an acceptor peer, an error should be thrown.

// At this point, we know the spec secret exists. If the status secret doesn't
Expand Down Expand Up @@ -258,14 +253,11 @@ func (r *PeeringDialerController) initiatePeering(ctx context.Context, peerName
}

// deletePeering is a helper function that calls the Consul api to delete a peering.
func (r *PeeringDialerController) deletePeering(ctx context.Context, peerName string) (*api.PeeringDeleteResponse, error) {
deleteReq := api.PeeringDeleteRequest{
Name: peerName,
}
resp, _, err := r.ConsulClient.Peerings().Delete(ctx, deleteReq, nil)
func (r *PeeringDialerController) deletePeering(ctx context.Context, peerName string) error {
_, err := r.ConsulClient.Peerings().Delete(ctx, peerName, nil)
if err != nil {
r.Log.Error(err, "failed to delete Peering from Consul", "name", peerName)
return nil, err
return err
}
return resp, nil
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package connectinject

import (
"context"
"net/http"
"testing"

logrtest "github.com/go-logr/logr/testing"
Expand Down Expand Up @@ -91,12 +90,9 @@ func TestReconcileDeletePeeringDialer(t *testing.T) {
require.False(t, resp.Requeue)

// After reconciliation, Consul should not have the peering.
readReq := api.PeeringReadRequest{Name: "dialer-deleted"}
peering, _, err := consulClient.Peerings().Read(context.Background(), readReq, nil)
var statusErr api.StatusError
require.ErrorAs(t, err, &statusErr)
require.Equal(t, http.StatusNotFound, statusErr.Code)
peering, _, err := consulClient.Peerings().Read(context.Background(), "dialer-deleted", nil)
require.Nil(t, peering)
require.NoError(t, err)
})
}
}
3 changes: 3 additions & 0 deletions control-plane/controller/configentry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@ func setupWithManager(mgr ctrl.Manager, resource client.Object, reconciler recon
}

func (r *ConfigEntryController) consulNamespace(configEntry capi.ConfigEntry, namespace string, globalResource bool) string {
if !r.EnableConsulNamespaces {
return ""
}
// ServiceIntentions have the appropriate Consul Namespace set on them as the value
// is defaulted by the webhook. These are then set on the ServiceIntentions config entry
// but not on the others. In case the ConfigEntry has the Consul Namespace set, we just
Expand Down
6 changes: 3 additions & 3 deletions control-plane/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.20220519230759-6167400b28c9
github.com/hashicorp/consul/api v1.10.1-0.20220525202017-d8d8c8603e9b
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
Expand Down Expand Up @@ -108,7 +108,7 @@ require (
golang.org/x/net v0.0.0-20211209124913-491a49abca63 // indirect
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 // indirect
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
Expand All @@ -129,7 +129,7 @@ require (
sigs.k8s.io/yaml v1.2.0 // indirect
)

replace github.com/hashicorp/consul/sdk v0.9.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220214194852-80dfcb1bcd68
replace github.com/hashicorp/consul/sdk v0.9.0 => github.com/hashicorp/consul/sdk v0.4.1-0.20220531155537-364758ef2f50

//replace github.com/hashicorp/consul/api v1.10.1-0.20220425143126-6d0162a58a94 => /Users/nitya/workspace/hashicorp/consul/api

Expand Down
11 changes: 6 additions & 5 deletions control-plane/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,11 @@ 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.20220519230759-6167400b28c9 h1:HWXZXOMz3EXKtpE+ETagwEtppdCywlcQ799oEpdxfxc=
github.com/hashicorp/consul/api v1.10.1-0.20220519230759-6167400b28c9/go.mod h1:ZlVrynguJKcYr54zGaDbaL3fOvKC9m72FhPvA8T35KQ=
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/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
github.com/hashicorp/consul/sdk v0.4.1-0.20220214194852-80dfcb1bcd68 h1:yw3OXf1OUgfnitE8rwnr+zaT9VluSgvrCHQGwSvA7V4=
github.com/hashicorp/consul/sdk v0.4.1-0.20220214194852-80dfcb1bcd68/go.mod h1:K9S7H8bLBwkBb2I4hq0Ddm4LCVGuhtenfzSTx2Y36RM=
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=
github.com/hashicorp/consul/sdk v0.8.0/go.mod h1:GBvyrGALthsZObzUGsfgHZQDXjg4lOjagTIwIR1vPms=
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
Expand Down Expand Up @@ -797,8 +797,9 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 h1:c8PlLMqBbOHoqtjteWm5/kbe6rNY2pbRfbIMVnepueo=
golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE=
Expand Down

0 comments on commit fa1936c

Please sign in to comment.