Skip to content

Commit

Permalink
fix(rds): DBCluster controller not recognizing changes in VpcSecurity…
Browse files Browse the repository at this point in the history
…Groups

Signed-off-by: Paul Schroeder <[email protected]>
  • Loading branch information
schroeder-paul committed Nov 18, 2022
1 parent 0f81db2 commit 989cc2d
Show file tree
Hide file tree
Showing 4 changed files with 235 additions and 9 deletions.
8 changes: 5 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ require (
github.com/mitchellh/copystructure v1.0.0
github.com/onsi/gomega v1.17.0
github.com/pkg/errors v0.9.1
github.com/samber/lo v1.35.0
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd
gopkg.in/alecthomas/kingpin.v2 v2.2.6
k8s.io/api v0.23.0
Expand Down Expand Up @@ -125,13 +126,14 @@ require (
go.uber.org/multierr v1.6.0 // indirect
go.uber.org/zap v1.19.1 // indirect
golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa // indirect
golang.org/x/mod v0.4.2 // indirect
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 // indirect
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57 // indirect
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff // indirect
golang.org/x/tools v0.1.8-0.20211029000441-d6a9af8af023 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
Expand All @@ -141,7 +143,7 @@ require (
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/square/go-jose.v2 v2.5.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/apiextensions-apiserver v0.23.0 // indirect
k8s.io/component-base v0.23.0 // indirect
k8s.io/klog/v2 v2.30.0 // indirect
Expand Down
16 changes: 12 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb
github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk=
github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc=
github.com/samber/lo v1.35.0 h1:GlT8CV1GE+v97Y7MLF1wXvX6mjoxZ+hi61tj/ZcQwY0=
github.com/samber/lo v1.35.0/go.mod h1:HLeWcJRRyLKp3+/XBJvOrerCQn9mhdKMHyd7IRlgeQ8=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc=
github.com/sirupsen/logrus v1.2.0/go.mod h1:LxeOpSwHxABJmUn/MG1IvRgCAasNZTLOkJPxbbu5VWo=
Expand Down Expand Up @@ -659,9 +661,10 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/thoas/go-funk v0.9.1 h1:O549iLZqPpTUQ10ykd26sZhzD+rmR5pWhuElrhbC20M=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tmc/grpc-websocket-proxy v0.0.0-20201229170055-e5319fda7802/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
Expand Down Expand Up @@ -739,6 +742,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 h1:3MTrJm4PyNL9NBqvYDSj3DHl46qQakyfqfWo4jgfaEM=
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
Expand All @@ -763,8 +768,9 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57 h1:LQmS1nU0twXLA96Kt7U9qtHJEbBk3z6Q0V4UXjZkpr4=
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
golang.org/x/net v0.0.0-20180530234432-1e491301e022/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down Expand Up @@ -994,8 +1000,9 @@ golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4f
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff h1:VX/uD7MK0AHXGiScH3fsieUQUcpmRERPDYtqZdJnA+Q=
golang.org/x/tools v0.1.6-0.20210820212750-d4cc65f0b2ff/go.mod h1:YD9qOF0M9xpSpdWTBbzEl5e/RnCefISl8E5Noe10jFM=
golang.org/x/tools v0.1.8-0.20211029000441-d6a9af8af023 h1:0c3L82FDQ5rt1bjTBlchS8t6RQ6299/+5bWMnRLh+uI=
golang.org/x/tools v0.1.8-0.20211029000441-d6a9af8af023/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
Expand Down Expand Up @@ -1154,8 +1161,9 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=
gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
33 changes: 31 additions & 2 deletions pkg/controller/rds/dbcluster/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ package dbcluster

import (
"context"
"sort"
"strconv"
"strings"

"github.com/google/go-cmp/cmp"

svcsdk "github.com/aws/aws-sdk-go/service/rds"
svcsdkapi "github.com/aws/aws-sdk-go/service/rds/rdsiface"
svcapitypes "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

svcapitypes "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"

"github.com/crossplane-contrib/provider-aws/apis/v1alpha1"

aws "github.com/crossplane-contrib/provider-aws/pkg/clients"
Expand Down Expand Up @@ -480,6 +482,10 @@ func isUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOutput)
return false, nil
}

if !isVPCSecurityGroupIDsUpToDate(cr, out) {
return false, nil
}

isScalingConfigurationUpToDate, err := isScalingConfigurationUpToDate(cr.Spec.ForProvider.ScalingConfiguration, out.DBClusters[0].ScalingConfigurationInfo)
if !isScalingConfigurationUpToDate {
return false, err
Expand Down Expand Up @@ -562,6 +568,29 @@ func isPortUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOut
return true
}

func isVPCSecurityGroupIDsUpToDate(cr *svcapitypes.DBCluster, out *svcsdk.DescribeDBClustersOutput) bool {
// AWS uses "sg-563ab33d" which ich really restrictive as the default, and it seems to use it even when it is
// patched (with "required") - might be race condition. Anyway with checking if there is a diff we can rectify and
// even make it configurable after creation.

actualGroups := out.DBClusters[0].VpcSecurityGroups
desiredIDs := cr.Spec.ForProvider.VPCSecurityGroupIDs

if len(desiredIDs) != len(actualGroups) {
return false
}

var actualIDs []string
for _, grp := range out.DBClusters[0].VpcSecurityGroups {
actualIDs = append(actualIDs, *grp.VpcSecurityGroupId)
}

sort.Strings(desiredIDs)
sort.Strings(actualIDs)

return cmp.Equal(desiredIDs, actualIDs)
}

func preUpdate(_ context.Context, cr *svcapitypes.DBCluster, obj *svcsdk.ModifyDBClusterInput) error {
obj.DBClusterIdentifier = aws.String(meta.GetExternalName(cr))
obj.ApplyImmediately = cr.Spec.ForProvider.ApplyImmediately
Expand Down
187 changes: 187 additions & 0 deletions pkg/controller/rds/dbcluster/setup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
package dbcluster

import (
"testing"

svcsdk "github.com/aws/aws-sdk-go/service/rds"
svcapitypes "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"

"github.com/google/go-cmp/cmp"
)

func ptr[T any](v T) *T {
return &v
}

func TestIsVPCSecurityGroupIDsUpToDate(t *testing.T) {
type args struct {
cr *svcapitypes.DBCluster
out *svcsdk.DescribeDBClustersOutput
}

type want struct {
isUpToDate bool
}

cases := map[string]struct {
args
want
}{
"NotAsMany": {
args: args{
cr: &svcapitypes.DBCluster{
Spec: svcapitypes.DBClusterSpec{
ForProvider: svcapitypes.DBClusterParameters{
CustomDBClusterParameters: svcapitypes.CustomDBClusterParameters{
VPCSecurityGroupIDs: []string{"sg-123", "sg-456"},
},
},
},
},
out: &svcsdk.DescribeDBClustersOutput{
DBClusters: []*svcsdk.DBCluster{
&svcsdk.DBCluster{
VpcSecurityGroups: []*svcsdk.VpcSecurityGroupMembership{
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-123"),
},
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-456"),
},
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-789"),
},
},
},
},
},
},
want: want{
isUpToDate: false,
},
},
"DesiredEmpty": {
args: args{
cr: &svcapitypes.DBCluster{
Spec: svcapitypes.DBClusterSpec{
ForProvider: svcapitypes.DBClusterParameters{
CustomDBClusterParameters: svcapitypes.CustomDBClusterParameters{
VPCSecurityGroupIDs: nil,
},
},
},
},
out: &svcsdk.DescribeDBClustersOutput{
DBClusters: []*svcsdk.DBCluster{
&svcsdk.DBCluster{
VpcSecurityGroups: []*svcsdk.VpcSecurityGroupMembership{
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-456"),
},
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-123"),
},
},
},
},
},
},
want: want{
isUpToDate: false,
},
},
"ActualEmpty": {
args: args{
cr: &svcapitypes.DBCluster{
Spec: svcapitypes.DBClusterSpec{
ForProvider: svcapitypes.DBClusterParameters{
CustomDBClusterParameters: svcapitypes.CustomDBClusterParameters{
VPCSecurityGroupIDs: []string{"sg-123", "sg-456"},
},
},
},
},
out: &svcsdk.DescribeDBClustersOutput{
DBClusters: []*svcsdk.DBCluster{
&svcsdk.DBCluster{
VpcSecurityGroups: nil,
},
},
},
},
want: want{
isUpToDate: false,
},
},
"Unsorted": {
args: args{
cr: &svcapitypes.DBCluster{
Spec: svcapitypes.DBClusterSpec{
ForProvider: svcapitypes.DBClusterParameters{
CustomDBClusterParameters: svcapitypes.CustomDBClusterParameters{
VPCSecurityGroupIDs: []string{"sg-123", "sg-456"},
},
},
},
},
out: &svcsdk.DescribeDBClustersOutput{
DBClusters: []*svcsdk.DBCluster{
&svcsdk.DBCluster{
VpcSecurityGroups: []*svcsdk.VpcSecurityGroupMembership{
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-456"),
},
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-123"),
},
},
},
},
},
},
want: want{
isUpToDate: true,
},
},
"Identical": {
args: args{
cr: &svcapitypes.DBCluster{
Spec: svcapitypes.DBClusterSpec{
ForProvider: svcapitypes.DBClusterParameters{
CustomDBClusterParameters: svcapitypes.CustomDBClusterParameters{
VPCSecurityGroupIDs: []string{"sg-123", "sg-456"},
},
},
},
},
out: &svcsdk.DescribeDBClustersOutput{
DBClusters: []*svcsdk.DBCluster{
&svcsdk.DBCluster{
VpcSecurityGroups: []*svcsdk.VpcSecurityGroupMembership{
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-123"),
},
&svcsdk.VpcSecurityGroupMembership{
VpcSecurityGroupId: ptr("sg-456"),
},
},
},
},
},
},
want: want{
isUpToDate: true,
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
isUpToDate := isVPCSecurityGroupIDsUpToDate(tc.args.cr, tc.args.out)

if diff := cmp.Diff(tc.want.isUpToDate, isUpToDate); diff != "" {
t.Errorf("r: -want, +got:\n%s", diff)
}
})
}
}

0 comments on commit 989cc2d

Please sign in to comment.