Skip to content

Commit

Permalink
fix: AWSNodeTemplate controller should not produce an error if no sec…
Browse files Browse the repository at this point in the history
…urityGroupSelector is specified (#3437)
  • Loading branch information
engedaam authored Feb 22, 2023
1 parent 39e6d68 commit a950b3a
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 4 deletions.
2 changes: 2 additions & 0 deletions pkg/cloudprovider/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,8 @@ var _ = Describe("Allocation", func() {
provider, err := v1alpha1.DeserializeProvider(provisioner.Spec.Provider.Raw)
Expect(err).ToNot(HaveOccurred())
provider.Context = aws.String("context-1234")
provider.SubnetSelector = map[string]string{"*": "*"}
provider.SecurityGroupSelector = map[string]string{"*": "*"}
provisioner = coretest.Provisioner(coretest.ProvisionerOptions{Provider: provider})
provisioner.SetDefaults(ctx)
ExpectApplied(ctx, env.Client, provisioner)
Expand Down
12 changes: 12 additions & 0 deletions pkg/controllers/nodetemplate/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,18 @@ var _ = Describe("AWSNodeTemplateController", func() {
})
})
Context("Security Groups Status", func() {
It("Should expect no errors when security groups are not in the AWSNodeTemplate", func() {
// TODO: Remove test for v1beta1, as security groups will be required
nodeTemplate.Spec.SecurityGroupSelector = nil
ExpectApplied(ctx, env.Client, nodeTemplate)
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate))
nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate)
securityGroupsIDs, _ := securityGroupProvider.List(ctx, nodeTemplate)
securityGroupsIDInStatus := lo.Map(nodeTemplate.Status.SecurityGroups, func(securitygroup v1alpha1.SecurityGroupStatus, _ int) string {
return securitygroup.ID
})
Expect(securityGroupsIDInStatus).To(Equal(securityGroupsIDs))
})
It("Should update AWSNodeTemplate status for Security Groups", func() {
ExpectApplied(ctx, env.Client, nodeTemplate)
ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate))
Expand Down
7 changes: 6 additions & 1 deletion pkg/fake/ec2api.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,9 @@ func (e *EC2API) DescribeSubnetsWithContext(ctx context.Context, input *ec2.Desc
},
},
}

if len(input.Filters) == 0 {
return nil, fmt.Errorf("InvalidParameterValue: The filter 'null' is invalid")
}
return &ec2.DescribeSubnetsOutput{Subnets: FilterDescribeSubnets(subnets, input.Filters)}, nil
}

Expand Down Expand Up @@ -429,6 +431,9 @@ func (e *EC2API) DescribeSecurityGroupsWithContext(ctx context.Context, input *e
},
},
}
if len(input.Filters) == 0 {
return nil, fmt.Errorf("InvalidParameterValue: The filter 'null' is invalid")
}
return &ec2.DescribeSecurityGroupsOutput{SecurityGroups: FilterDescribeSecurtyGroups(sgs, input.Filters)}, nil
}

Expand Down
10 changes: 8 additions & 2 deletions pkg/providers/securitygroup/securitygroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func NewProvider(ec2api ec2iface.EC2API) *Provider {
return &Provider{
ec2api: ec2api,
cm: pretty.NewChangeMonitor(),
// TODO: Remove cahce for v1bata1, utlize resolved security groups from the AWSNodeTemplate.status
// TODO: Remove cache for v1beta1, utilize resolved security groups from the AWSNodeTemplate.status
cache: cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval),
}
}
Expand All @@ -55,7 +55,13 @@ func (p *Provider) List(ctx context.Context, nodeTemplate *v1alpha1.AWSNodeTempl
p.Lock()
defer p.Unlock()
// Get SecurityGroups
securityGroups, err := p.getSecurityGroups(ctx, p.getFilters(nodeTemplate))
// TODO: When removing custom launchTemplates for v1beta1, security groups will be required.
// The check will not be necessary
filters := p.getFilters(nodeTemplate)
if len(filters) == 0 {
return []string{}, nil
}
securityGroups, err := p.getSecurityGroups(ctx, filters)
if err != nil {
return nil, err
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/providers/subnet/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ func (p *Provider) List(ctx context.Context, nodeTemplate *v1alpha1.AWSNodeTempl
p.Lock()
defer p.Unlock()
filters := getFilters(nodeTemplate)
if len(filters) == 0 {
return []*ec2.Subnet{}, nil
}
hash, err := hashstructure.Hash(filters, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
if err != nil {
return nil, err
Expand Down
7 changes: 6 additions & 1 deletion pkg/test/awsnodetemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import (
func AWSNodeTemplate(overrides ...v1alpha1.AWSNodeTemplateSpec) *v1alpha1.AWSNodeTemplate {
return &v1alpha1.AWSNodeTemplate{
ObjectMeta: test.ObjectMeta(),
Spec: test.MustMerge(v1alpha1.AWSNodeTemplateSpec{}, overrides...),
Spec: test.MustMerge(v1alpha1.AWSNodeTemplateSpec{
AWS: v1alpha1.AWS{
SubnetSelector: map[string]string{"*": "*"},
SecurityGroupSelector: map[string]string{"*": "*"},
},
}, overrides...),
}
}

0 comments on commit a950b3a

Please sign in to comment.