Skip to content

Commit 5b22802

Browse files
sedefsavassedefsavas
authored
Merge pull request #3257 from Ankitasw/resource-ref-lookup
Added lookup for fields of type AWSResourcesReference
2 parents 2c454a0 + 369bb69 commit 5b22802

22 files changed

+280
-160
lines changed

‎api/v1alpha4/conversion.go

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,10 @@ limitations under the License.
1717
package v1alpha4
1818

1919
import (
20-
apiconversion "k8s.io/apimachinery/pkg/conversion"
21-
conversion "k8s.io/apimachinery/pkg/conversion"
22-
v1beta1 "sigs.k8s.io/cluster-api-provider-aws/api/v1beta1"
23-
clusterv1alpha4 "sigs.k8s.io/cluster-api/api/v1alpha4"
24-
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
20+
"k8s.io/apimachinery/pkg/conversion"
21+
"sigs.k8s.io/cluster-api-provider-aws/api/v1beta1"
2522
)
2623

27-
// Convert_v1alpha4_APIEndpoint_To_v1beta1_APIEndpoint .
28-
func Convert_v1alpha4_ObjectMeta_To_v1beta1_ObjectMeta(in *clusterv1alpha4.ObjectMeta, out *clusterv1.ObjectMeta, s apiconversion.Scope) error {
29-
return clusterv1alpha4.Convert_v1alpha4_ObjectMeta_To_v1beta1_ObjectMeta(in, out, s)
30-
}
31-
32-
// Convert_v1beta1_APIEndpoint_To_v1alpha4_APIEndpoint .
33-
func Convert_v1beta1_ObjectMeta_To_v1alpha4_ObjectMeta(in *clusterv1.ObjectMeta, out *clusterv1alpha4.ObjectMeta, s apiconversion.Scope) error {
34-
return clusterv1alpha4.Convert_v1beta1_ObjectMeta_To_v1alpha4_ObjectMeta(in, out, s)
35-
}
36-
3724
func Convert_v1beta1_AWSClusterSpec_To_v1alpha4_AWSClusterSpec(in *v1beta1.AWSClusterSpec, out *AWSClusterSpec, s conversion.Scope) error {
3825
return autoConvert_v1beta1_AWSClusterSpec_To_v1alpha4_AWSClusterSpec(in, out, s)
3926
}

‎api/v1beta1/awsmachine_webhook.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
)
3131

3232
// log is for logging in this package.
33-
var _ = logf.Log.WithName("awsmachine-resource")
33+
var log = logf.Log.WithName("awsmachine-resource")
3434

3535
func (r *AWSMachine) SetupWebhookWithManager(mgr ctrl.Manager) error {
3636
return ctrl.NewWebhookManagedBy(mgr).
@@ -253,6 +253,9 @@ func (r *AWSMachine) validateAdditionalSecurityGroups() field.ErrorList {
253253
if len(additionalSecurityGroup.Filters) > 0 && additionalSecurityGroup.ID != nil {
254254
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.additionalSecurityGroups"), "only one of ID or Filters may be specified, specifying both is forbidden"))
255255
}
256+
if additionalSecurityGroup.ARN != nil {
257+
log.Info("ARN field is deprecated and is no operation function.")
258+
}
256259
}
257260
return allErrs
258261
}

‎api/v1beta1/types.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,17 @@ import (
2222
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2323
)
2424

25-
// AWSResourceReference is a reference to a specific AWS resource by ID, ARN, or filters.
26-
// Only one of ID, ARN or Filters may be specified. Specifying more than one will result in
25+
// AWSResourceReference is a reference to a specific AWS resource by ID or filters.
26+
// Only one of ID or Filters may be specified. Specifying more than one will result in
2727
// a validation error.
2828
type AWSResourceReference struct {
2929
// ID of resource
3030
// +optional
3131
ID *string `json:"id,omitempty"`
3232

33-
// ARN of resource
33+
// ARN of resource.
3434
// +optional
35+
// Deprecated: This field has no function and is going to be removed in the next release.
3536
ARN *string `json:"arn,omitempty"`
3637

3738
// Filters is a set of key/value pairs used to identify a resource

‎config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinepools.yaml

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,12 +1018,13 @@ spec:
10181018
groups defined at the cluster level or in the actuator.
10191019
items:
10201020
description: AWSResourceReference is a reference to a specific
1021-
AWS resource by ID, ARN, or filters. Only one of ID, ARN or
1022-
Filters may be specified. Specifying more than one will result
1023-
in a validation error.
1021+
AWS resource by ID or filters. Only one of ID or Filters may
1022+
be specified. Specifying more than one will result in a validation
1023+
error.
10241024
properties:
10251025
arn:
1026-
description: ARN of resource
1026+
description: 'ARN of resource. Deprecated: This field has
1027+
no function and is going to be removed in the next release.'
10271028
type: string
10281029
filters:
10291030
description: 'Filters is a set of key/value pairs used to
@@ -1263,12 +1264,12 @@ spec:
12631264
description: Subnets is an array of subnet configurations
12641265
items:
12651266
description: AWSResourceReference is a reference to a specific AWS
1266-
resource by ID, ARN, or filters. Only one of ID, ARN or Filters
1267-
may be specified. Specifying more than one will result in a validation
1268-
error.
1267+
resource by ID or filters. Only one of ID or Filters may be specified.
1268+
Specifying more than one will result in a validation error.
12691269
properties:
12701270
arn:
1271-
description: ARN of resource
1271+
description: 'ARN of resource. Deprecated: This field has no
1272+
function and is going to be removed in the next release.'
12721273
type: string
12731274
filters:
12741275
description: 'Filters is a set of key/value pairs used to identify

‎config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachines.yaml

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -987,12 +987,12 @@ spec:
987987
change too.
988988
items:
989989
description: AWSResourceReference is a reference to a specific AWS
990-
resource by ID, ARN, or filters. Only one of ID, ARN or Filters
991-
may be specified. Specifying more than one will result in a validation
992-
error.
990+
resource by ID or filters. Only one of ID or Filters may be specified.
991+
Specifying more than one will result in a validation error.
993992
properties:
994993
arn:
995-
description: ARN of resource
994+
description: 'ARN of resource. Deprecated: This field has no
995+
function and is going to be removed in the next release.'
996996
type: string
997997
filters:
998998
description: 'Filters is a set of key/value pairs used to identify
@@ -1245,7 +1245,8 @@ spec:
12451245
If not specified, the cluster subnet will be used.
12461246
properties:
12471247
arn:
1248-
description: ARN of resource
1248+
description: 'ARN of resource. Deprecated: This field has no function
1249+
and is going to be removed in the next release.'
12491250
type: string
12501251
filters:
12511252
description: 'Filters is a set of key/value pairs used to identify

‎config/crd/bases/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yaml

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -789,12 +789,14 @@ spec:
789789
the attached security groups might change too.
790790
items:
791791
description: AWSResourceReference is a reference to a specific
792-
AWS resource by ID, ARN, or filters. Only one of ID, ARN
793-
or Filters may be specified. Specifying more than one
794-
will result in a validation error.
792+
AWS resource by ID or filters. Only one of ID or Filters
793+
may be specified. Specifying more than one will result
794+
in a validation error.
795795
properties:
796796
arn:
797-
description: ARN of resource
797+
description: 'ARN of resource. Deprecated: This field
798+
has no function and is going to be removed in the
799+
next release.'
798800
type: string
799801
filters:
800802
description: 'Filters is a set of key/value pairs used
@@ -1059,7 +1061,9 @@ spec:
10591061
be used.
10601062
properties:
10611063
arn:
1062-
description: ARN of resource
1064+
description: 'ARN of resource. Deprecated: This field
1065+
has no function and is going to be removed in the next
1066+
release.'
10631067
type: string
10641068
filters:
10651069
description: 'Filters is a set of key/value pairs used

‎controllers/awsmachine_controller_unit_test.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,7 @@ func TestAWSMachineReconciler(t *testing.T) {
450450
},
451451
}
452452
ec2Svc.EXPECT().UpdateInstanceSecurityGroups(instance.ID, []string{"sg-2345"})
453+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return([]string{"sg-2345"}, nil)
453454

454455
_, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
455456
expectConditions(g, ms.AWSMachine, []conditionAssertion{{conditionType: infrav1.SecurityGroupsReadyCondition, status: corev1.ConditionTrue}})
@@ -463,6 +464,7 @@ func TestAWSMachineReconciler(t *testing.T) {
463464
instanceCreate(t, g)
464465
getCoreSecurityGroups(t, g)
465466

467+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
466468
ec2Svc.EXPECT().UpdateInstanceSecurityGroups(gomock.Any(), gomock.Any()).Times(0)
467469
if _, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs); err != nil {
468470
_ = fmt.Errorf("reconcileNormal reutrned an error during test")
@@ -480,6 +482,7 @@ func TestAWSMachineReconciler(t *testing.T) {
480482
ms.AWSMachine.Spec.AdditionalTags = infrav1.Tags{"kind": "alicorn"}
481483
cs.AWSCluster.Spec.AdditionalTags = infrav1.Tags{"colour": "lavender"}
482484

485+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
483486
ec2Svc.EXPECT().UpdateResourceTags(
484487
gomock.Any(),
485488
map[string]string{
@@ -510,6 +513,7 @@ func TestAWSMachineReconciler(t *testing.T) {
510513

511514
ms.AWSMachine.Spec.AdditionalTags = infrav1.Tags{"rootDeviceID": "id1", "rootDeviceSize": "30"}
512515

516+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
513517
ec2Svc.EXPECT().UpdateResourceTags(
514518
gomock.Any(),
515519
map[string]string{
@@ -536,6 +540,7 @@ func TestAWSMachineReconciler(t *testing.T) {
536540
secretSvc.EXPECT().UserData(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1)
537541
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
538542
secretSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return("test", int32(1), nil).Times(1)
543+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
539544
}
540545

541546
t.Run("should set instance to stopping and unready", func(t *testing.T) {
@@ -646,6 +651,7 @@ func TestAWSMachineReconciler(t *testing.T) {
646651
secretSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return("test", int32(1), nil).Times(1)
647652
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
648653
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
654+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
649655

650656
_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
651657
g.Expect(err).To(BeNil())
@@ -671,6 +677,7 @@ func TestAWSMachineReconciler(t *testing.T) {
671677
secretSvc.EXPECT().Create(gomock.Any(), gomock.Any()).Return("test", int32(1), nil).Times(1)
672678
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
673679
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
680+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
674681

675682
_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
676683
g.Expect(err).To(BeNil())
@@ -688,6 +695,7 @@ func TestAWSMachineReconciler(t *testing.T) {
688695
ms.AWSMachine.Spec.CloudInit.InsecureSkipSecretsManager = true
689696
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
690697
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
698+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
691699
reconciler.elbServiceFactory = func(elbScope scope.ELBScope) services.ELBInterface {
692700
return elbSvc
693701
}
@@ -918,8 +926,6 @@ func TestAWSMachineReconciler(t *testing.T) {
918926
ms.AWSMachine.Annotations = map[string]string{SecurityGroupsLastAppliedAnnotation: "{\"tag\":\"tag1\"}"}
919927
ms.AWSMachine.Spec.AdditionalSecurityGroups = []infrav1.AWSResourceReference{
920928
{
921-
ID: aws.String("sg-1"),
922-
ARN: aws.String("arn-1"),
923929
Filters: []infrav1.Filter{
924930
{
925931
Name: "example-name",
@@ -930,7 +936,7 @@ func TestAWSMachineReconciler(t *testing.T) {
930936
}
931937

932938
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil)
933-
ec2Svc.EXPECT().GetFilteredSecurityGroupID(gomock.Any()).Return("sg-1", errors.New("failed to get filtered SGs"))
939+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return([]string{"sg-1"}, errors.New("failed to get filtered SGs"))
934940

935941
_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
936942
g.Expect(err).To(BeNil())
@@ -950,20 +956,18 @@ func TestAWSMachineReconciler(t *testing.T) {
950956
ms.AWSMachine.Annotations = map[string]string{SecurityGroupsLastAppliedAnnotation: "{\"tag\":\"tag1\"}"}
951957
ms.AWSMachine.Spec.AdditionalSecurityGroups = []infrav1.AWSResourceReference{
952958
{
953-
ID: aws.String("sg-1"),
954-
ARN: aws.String("arn-1"),
955959
Filters: []infrav1.Filter{
956960
{
957-
Name: "example-name",
958-
Values: []string{"example-value"},
961+
Name: "id",
962+
Values: []string{"sg-1"},
959963
},
960964
},
961965
},
962966
}
963967

964-
ec2Svc.EXPECT().GetFilteredSecurityGroupID(gomock.Any()).Return("sg-1", nil)
965968
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil)
966969
ec2Svc.EXPECT().UpdateInstanceSecurityGroups(gomock.Any(), gomock.Any()).Return(errors.New("failed to update security groups"))
970+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return([]string{"sg-1"}, nil)
967971

968972
_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
969973
g.Expect(err).ToNot(BeNil())
@@ -996,6 +1000,7 @@ func TestAWSMachineReconciler(t *testing.T) {
9961000
secretSvc.EXPECT().UserData(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1)
9971001
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
9981002
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1003+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
9991004

10001005
ms.AWSMachine.ObjectMeta.Labels = map[string]string{
10011006
clusterv1.MachineControlPlaneLabelName: "",
@@ -1038,6 +1043,7 @@ func TestAWSMachineReconciler(t *testing.T) {
10381043
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).
10391044
Return(map[string][]string{"eid": {}}, nil).Times(1)
10401045
secretSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1)
1046+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
10411047
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
10421048
_, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
10431049
})
@@ -1123,6 +1129,7 @@ func TestAWSMachineReconciler(t *testing.T) {
11231129
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).
11241130
Return(map[string][]string{"eid": {}}, nil).Times(1)
11251131
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1132+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
11261133
secretSvc.EXPECT().Delete(gomock.Any()).Return(nil).MaxTimes(0)
11271134
_, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
11281135
})
@@ -1205,6 +1212,7 @@ func TestAWSMachineReconciler(t *testing.T) {
12051212
secretSvc.EXPECT().UserData(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).Times(1)
12061213
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
12071214
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1215+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
12081216

12091217
_, err := reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
12101218

@@ -1251,6 +1259,7 @@ func TestAWSMachineReconciler(t *testing.T) {
12511259
ec2Svc.EXPECT().CreateInstance(gomock.Any(), gomock.Any(), gomock.Any()).Return(instance, nil).AnyTimes()
12521260
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
12531261
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1262+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
12541263

12551264
ms.AWSMachine.ObjectMeta.Labels = map[string]string{
12561265
clusterv1.MachineControlPlaneLabelName: "",
@@ -1298,6 +1307,7 @@ func TestAWSMachineReconciler(t *testing.T) {
12981307
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
12991308
objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).Times(1)
13001309
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1310+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
13011311

13021312
_, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
13031313
})
@@ -1379,6 +1389,7 @@ func TestAWSMachineReconciler(t *testing.T) {
13791389
instance.State = infrav1.InstanceStateRunning
13801390
ec2Svc.EXPECT().GetInstanceSecurityGroups(gomock.Any()).Return(map[string][]string{"eid": {}}, nil).Times(1)
13811391
ec2Svc.EXPECT().GetCoreSecurityGroups(gomock.Any()).Return([]string{}, nil).Times(1)
1392+
ec2Svc.EXPECT().GetAdditionalSecurityGroupsIDs(gomock.Any()).Return(nil, nil)
13821393
objectStoreSvc.EXPECT().Delete(gomock.Any()).Return(nil).MaxTimes(0)
13831394
_, _ = reconciler.reconcileNormal(context.Background(), ms, cs, cs, cs, cs)
13841395
})

‎controllers/awsmachine_security_groups.go

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (r *AWSMachineReconciler) ensureSecurityGroups(ec2svc service.EC2Interface,
4949
return false, err
5050
}
5151

52-
additionalSecurityGroupsIDs, err := r.getAdditionalSecurityGroupsIDs(ec2svc, additional)
52+
additionalSecurityGroupsIDs, err := ec2svc.GetAdditionalSecurityGroupsIDs(additional)
5353
if err != nil {
5454
return false, nil // nolint:nilerr
5555
}
@@ -121,24 +121,3 @@ func (r *AWSMachineReconciler) securityGroupsChanged(annotation map[string]inter
121121

122122
return false, res
123123
}
124-
125-
func (r *AWSMachineReconciler) getAdditionalSecurityGroupsIDs(ec2svc service.EC2Interface, securitygroups []infrav1.AWSResourceReference) ([]string, error) {
126-
additionalSecurityGroupsIDs := []string{}
127-
128-
for _, sg := range securitygroups {
129-
if sg.ID != nil {
130-
additionalSecurityGroupsIDs = append(additionalSecurityGroupsIDs, *sg.ID)
131-
}
132-
133-
if sg.Filters != nil {
134-
id, err := ec2svc.GetFilteredSecurityGroupID(sg)
135-
if err != nil {
136-
return nil, err
137-
}
138-
139-
additionalSecurityGroupsIDs = append(additionalSecurityGroupsIDs, id)
140-
}
141-
}
142-
143-
return additionalSecurityGroupsIDs, nil
144-
}

0 commit comments

Comments
 (0)