-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(aws-codedeploy): add instance tag filter support for server Deployment Groups #824
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,41 @@ class ImportedServerDeploymentGroupRef extends ServerDeploymentGroupRef { | |
} | ||
} | ||
|
||
/** | ||
* Represents a group of instance tags. | ||
* An instance will match a group if it has a tag matching | ||
* any of the group's tags by key and any of the provided values - | ||
* in other words, tag groups follow 'or' semantics. | ||
* If the value for a given key is an empty array, | ||
* an instance will match when it has a tag with the given key, | ||
* regardless of the value. | ||
* If the key is an empty string, any tag, | ||
* regardless of its key, with any of the given values, will match. | ||
*/ | ||
export type InstanceTagGroup = {[key: string]: string[]}; | ||
|
||
/** | ||
* Represents a set of instance tag groups. | ||
* An instance will match a set if it matches all of the groups in the set - | ||
* in other words, sets follow 'and' semantics. | ||
* You can have a maximum of 3 tag groups inside a set. | ||
*/ | ||
export class InstanceTagSet { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this type at all needed? Makes usage more convoluted without adding much value. I would expect: new DeploymentGroup(this, 'gd', {
ec2InstanceTags: {
'key1': [ 'tag1', 'tag2' ]
}
}); Also, since this is an array, we have seen it's useful to have a mutator, so add a method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you need to have 'and' and 'or' semantics represented. Have you seen the example from the ReadMe I gave? onPremiseInstanceTags: new codedeploy.InstanceTagSet(
// only instances with tags (key1=v1 or key1=v2) AND key2=v3 will match this set
{
'key1': ['v1', 'v2'],
},
{
'key2': ['v3'],
},
), |
||
private readonly _instanceTagGroups: InstanceTagGroup[]; | ||
|
||
constructor(...instanceTagGroups: InstanceTagGroup[]) { | ||
if (instanceTagGroups.length > 3) { | ||
throw new Error('An instance tag set can have a maximum of 3 instance tag groups, ' + | ||
`but ${instanceTagGroups.length} were provided`); | ||
} | ||
this._instanceTagGroups = instanceTagGroups; | ||
} | ||
|
||
public get instanceTagGroups(): InstanceTagGroup[] { | ||
return this._instanceTagGroups.slice(); | ||
} | ||
} | ||
|
||
/** | ||
* Construction properties for {@link ServerDeploymentGroup}. | ||
*/ | ||
|
@@ -153,6 +188,20 @@ export interface ServerDeploymentGroupProps { | |
* @default the Deployment Group will not have a load balancer defined | ||
*/ | ||
loadBalancer?: codedeploylb.ILoadBalancer; | ||
|
||
/* | ||
* All EC2 instances matching the given set of tags when a deployment occurs will be added to this Deployment Group. | ||
* | ||
* @default no additional EC2 instances will be added to the Deployment Group | ||
*/ | ||
ec2InstanceTags?: InstanceTagSet; | ||
|
||
/** | ||
* All on-premise instances matching the given set of tags when a deployment occurs will be added to this Deployment Group. | ||
* | ||
* @default no additional on-premise instances will be added to the Deployment Group | ||
*/ | ||
onPremiseInstanceTags?: InstanceTagSet; | ||
} | ||
|
||
/** | ||
|
@@ -204,6 +253,8 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupRef { | |
: { | ||
deploymentOption: 'WITH_TRAFFIC_CONTROL', | ||
}, | ||
ec2TagSet: this.ec2TagSet(props.ec2InstanceTags), | ||
onPremisesTagSet: this.onPremiseTagSet(props.onPremiseInstanceTags), | ||
}); | ||
|
||
this.deploymentGroupName = resource.deploymentGroupName; | ||
|
@@ -284,6 +335,75 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupRef { | |
}; | ||
} | ||
} | ||
|
||
private ec2TagSet(tagSet?: InstanceTagSet): | ||
cloudformation.DeploymentGroupResource.EC2TagSetProperty | undefined { | ||
if (!tagSet || tagSet.instanceTagGroups.length === 0) { | ||
return undefined; | ||
} | ||
|
||
return { | ||
ec2TagSetList: tagSet.instanceTagGroups.map(tagGroup => { | ||
return { | ||
ec2TagGroup: this.tagGroup2TagsArray(tagGroup) as | ||
cloudformation.DeploymentGroupResource.EC2TagFilterProperty[], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you need two distinct types here ( |
||
}; | ||
}), | ||
}; | ||
} | ||
|
||
private onPremiseTagSet(tagSet?: InstanceTagSet): | ||
cloudformation.DeploymentGroupResource.OnPremisesTagSetProperty | undefined { | ||
if (!tagSet || tagSet.instanceTagGroups.length === 0) { | ||
return undefined; | ||
} | ||
|
||
return { | ||
onPremisesTagSetList: tagSet.instanceTagGroups.map(tagGroup => { | ||
return { | ||
onPremisesTagGroup: this.tagGroup2TagsArray(tagGroup) as | ||
cloudformation.DeploymentGroupResource.TagFilterProperty[], | ||
}; | ||
}), | ||
}; | ||
} | ||
|
||
private tagGroup2TagsArray(tagGroup: InstanceTagGroup): any[] { | ||
const tagsInGroup = []; | ||
for (const tagKey in tagGroup) { | ||
if (tagGroup.hasOwnProperty(tagKey)) { | ||
const tagValues = tagGroup[tagKey]; | ||
if (tagKey.length > 0) { | ||
if (tagValues.length > 0) { | ||
for (const tagValue of tagValues) { | ||
tagsInGroup.push({ | ||
key: tagKey, | ||
value: tagValue, | ||
type: 'KEY_AND_VALUE', | ||
}); | ||
} | ||
} else { | ||
tagsInGroup.push({ | ||
key: tagKey, | ||
type: 'KEY_ONLY', | ||
}); | ||
} | ||
} else { | ||
if (tagValues.length > 0) { | ||
for (const tagValue of tagValues) { | ||
tagsInGroup.push({ | ||
value: tagValue, | ||
type: 'VALUE_ONLY', | ||
}); | ||
} | ||
} else { | ||
throw new Error('Cannot specify both an empty key and no values for an instance tag filter'); | ||
} | ||
} | ||
} | ||
} | ||
return tagsInGroup; | ||
} | ||
} | ||
|
||
function deploymentGroupName2Arn(applicationName: string, deploymentGroupName: string): string { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how this type will be exported through jsii. I don't think it's really needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rix0rrr claims it should be fine. We have types like these already in the CDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this works. Don't know the exact output, but the type alias is transparent to jsii users