Skip to content

Commit

Permalink
(MINOR) Fix for AssertEC2TagValue method (#24)
Browse files Browse the repository at this point in the history 
### SUMMARY
* Fixed bug in  `aws.AssertEC2TagValue` method.
* Removed deprecated (and broken) `aws.AssertEC2TagValueE` method.

### REMOVED
* The deprecated AssertEC2TagValueE method has been removed. It was broken and would not have worked.

### FIXES
* The AssertEC2TagValue method now correctly includes the specified Instance ID in the filter list. Previously it did not include this, causing it to assert against all instances in a region.

### MISC
* The tests for the `aws.AssertEC2TagValue` method have been updated to use the `gomock` mocks rather than the hand written ones.

### Contribution Checklist
* [X] All changes have been reflected in comparable unit test changes or additions.
* [ ] (N/A) Any interactions with third party clients are done via interface types rather than direct interactions.
* [ ] (N/A) All new functions follow the required naming standard.
* [ ] (N/A) All new functions follow the required signature standards.
  • Loading branch information
@yardbirdsax
yardbirdsax authored Jan 21, 2022
1 parent 632881c commit 372195b
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 113 deletions.
47 changes: 14 additions & 33 deletions pkg/aws/ec2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ import (
"github.com/stretchr/testify/assert"
)

const (
resourceIDFilterName string = "resource-id"
resourceTypeFilterName string = "resource-type"
resourceTypeFilterValueInstance string = "instance"
)

type EC2Client interface {
DescribeInstances(context.Context, *ec2.DescribeInstancesInput, ...func(*ec2.Options)) (*ec2.DescribeInstancesOutput, error)

Expand Down Expand Up @@ -48,7 +54,7 @@ type AssertEC2TagValueInput struct {
TagName string
// The value of the tag to assert.
Value string
// The Instance ID that the tag mustbe set on.
// The Instance ID that the method will assert has a tag with the specified tag name and the specified value.
InstanceID string
}

Expand Down Expand Up @@ -115,14 +121,19 @@ func AssertEC2VolumeEncrypted(t *testing.T, ctx context.Context, client EC2Clien

// AssertEC2TagValue asserts that an EC2 instance has a tag with the given value.
func AssertEC2TagValue(t *testing.T, ctx context.Context, client EC2Client, input AssertEC2TagValueInput) {
resourceTypeFilterName := "resource-type"
resourceTypeFilterValue := "instance"
resourceTypeFilterName := resourceTypeFilterName
resourceTypeFilterValue := resourceTypeFilterValueInstance
resourceIDFilterName := resourceIDFilterName
describeTagsInput := &ec2.DescribeTagsInput{
Filters: []types.Filter{
{
Name: &resourceTypeFilterName,
Values: []string{resourceTypeFilterValue},
},
{
Name: &resourceIDFilterName,
Values: []string{input.InstanceID},
},
},
}
describeTagsOutput, err := client.DescribeTags(ctx, describeTagsInput)
Expand All @@ -139,36 +150,6 @@ func AssertEC2TagValue(t *testing.T, ctx context.Context, client EC2Client, inpu
assert.True(t, hasMatch, "Tag with key '%s' does not exist.", input.TagName)
}

// AssertEC2TagValueE asserts that an EC2 instance has a given tag with the given value.
// This func is deprecated in favor of the AssertEC2TagValue function.
func AssertEC2TagValueE(ctx context.Context, client EC2Client, input AssertEC2TagValueEInput) (assertion bool, err error) {
resourceTypeFilterName := "resource-type"
resourceTypeFilterValue := "instance"
describeTagsInput := &ec2.DescribeTagsInput{
Filters: []types.Filter{
{
Name: &resourceTypeFilterName,
Values: []string{resourceTypeFilterValue},
},
},
}
describeTagsOutput, err := client.DescribeTags(ctx, describeTagsInput)
if err != nil {
return false, err
}
hasTagMatch := false
for _, tag := range describeTagsOutput.Tags {
tagKey := tag.Key
tagValue := tag.Value
if *tagKey == input.TagName {
if *tagValue == input.Value {
hasTagMatch = true
}
}
}
return hasTagMatch, nil
}

func getEC2InstanceByInstanceIDE(ctx context.Context, client EC2Client, InstanceID string) (types.Instance, error) {
describeInstancesInput := &ec2.DescribeInstancesInput{
InstanceIds: []string{InstanceID},
Expand Down
115 changes: 35 additions & 80 deletions pkg/aws/ec2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,61 +489,38 @@ func TestAssertEC2TagValue_NoMatch(t *testing.T) {
},
},
}
clientMock := &EC2ClientMock{
DescribeTagsOutput: describeTagsOutput,
Test: t,
}
describeTagsInput := AssertEC2TagValueInput{
ctrl := gomock.NewController(t)
clientMock := mock.NewMockEC2Client(ctrl)
assertEC2TagValueInput := AssertEC2TagValueInput{
TagName: tagName,
Value: tagValue,
InstanceID: instanceID,
}
ctx := context.Background()
fakeTest := &testing.T{}

// Test
AssertEC2TagValue(fakeTest, ctx, clientMock, describeTagsInput)
assert.True(t, fakeTest.Failed(), "AssertEC2TagValue did not fail the test when the tag value did not match.")
}

func TestAssertEC2TagValueE_NoMatch(t *testing.T) {
// Setup
instanceID := "i546acas321sd"
tagName := "MyTag"
tagValue := "TagValue"
wrongTagValue := "OtherValue"
nextToken := ""
describeTagsOutput := &ec2.DescribeTagsOutput{
NextToken: &nextToken,
Tags: []types.TagDescription{
resourceTypeFilterName := resourceTypeFilterName
resourceTypeFilterValue := resourceTypeFilterValueInstance
resourceIDFilterName := resourceIDFilterName
describeTagsInput := &ec2.DescribeTagsInput{
Filters: []types.Filter{
{
ResourceId: &instanceID,
ResourceType: types.ResourceTypeInstance,
Key: &tagName,
Value: &wrongTagValue,
Name: &resourceTypeFilterName,
Values: []string{resourceTypeFilterValue},
},
{
Name: &resourceIDFilterName,
Values: []string{instanceID},
},
},
}
clientMock := &EC2ClientMock{
DescribeTagsOutput: describeTagsOutput,
Test: t,
}
describeTagsInput := AssertEC2TagValueEInput{
TagName: tagName,
Value: tagValue,
InstanceID: instanceID,
}
ctx := context.Background()
fakeTest := &testing.T{}
clientMock.EXPECT().DescribeTags(ctx, describeTagsInput).Times(1).Return(describeTagsOutput, nil)

// Test
result, err := AssertEC2TagValueE(ctx, clientMock, describeTagsInput)
if err != nil {
t.Fatal(err)
}
assert.False(t, result, "AssertEC2TagValueE returned 'true' when tag value did not match.")
AssertEC2TagValue(fakeTest, ctx, clientMock, assertEC2TagValueInput)
assert.True(t, fakeTest.Failed(), "AssertEC2TagValue did not fail the test when the tag value did not match.")
}

func TestAssertEC2TagValueE_Match(t *testing.T) {
func TestAssertEC2TagValue_Match(t *testing.T) {
// Setup
instanceID := "i546acas321sd"
tagName := "MyTag"
Expand All @@ -560,56 +537,34 @@ func TestAssertEC2TagValueE_Match(t *testing.T) {
},
},
}
clientMock := &EC2ClientMock{
DescribeTagsOutput: describeTagsOutput,
Test: t,
}
describeTagsInput := AssertEC2TagValueEInput{
assertEC2TagValueInput := AssertEC2TagValueInput{
TagName: tagName,
Value: tagValue,
InstanceID: instanceID,
}
ctx := context.Background()

// Test
result, err := AssertEC2TagValueE(ctx, clientMock, describeTagsInput)
if err != nil {
t.Fatal(err)
}
assert.True(t, result, "AssertEC2TagValueE returned 'false' when tag value matched.")
}

func TestAssertEC2TagValue_Match(t *testing.T) {
// Setup
instanceID := "i546acas321sd"
tagName := "MyTag"
tagValue := "TagValue"
nextToken := ""
describeTagsOutput := &ec2.DescribeTagsOutput{
NextToken: &nextToken,
Tags: []types.TagDescription{
ctrl := gomock.NewController(t)
clientMock := mock.NewMockEC2Client(ctrl)
resourceTypeFilterName := resourceTypeFilterName
resourceTypeFilterValue := resourceTypeFilterValueInstance
resourceIDFilterName := resourceIDFilterName
describeTagsInput := &ec2.DescribeTagsInput{
Filters: []types.Filter{
{
ResourceId: &instanceID,
ResourceType: types.ResourceTypeInstance,
Key: &tagName,
Value: &tagValue,
Name: &resourceTypeFilterName,
Values: []string{resourceTypeFilterValue},
},
{
Name: &resourceIDFilterName,
Values: []string{instanceID},
},
},
}
clientMock := &EC2ClientMock{
DescribeTagsOutput: describeTagsOutput,
Test: t,
}
describeTagsInput := AssertEC2TagValueInput{
TagName: tagName,
Value: tagValue,
InstanceID: instanceID,
}
ctx := context.Background()
fakeTest := &testing.T{}
clientMock.EXPECT().DescribeTags(ctx, describeTagsInput).Times(1).Return(describeTagsOutput, nil)

// Test
AssertEC2TagValue(fakeTest, ctx, clientMock, describeTagsInput)
AssertEC2TagValue(fakeTest, ctx, clientMock, assertEC2TagValueInput)
assert.False(t, fakeTest.Failed(), "AssertEC2TagValue failed the test when tag value matched.")
}

Expand Down

0 comments on commit 372195b

Please sign in to comment.