-
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
AWS Tags Support #91
Comments
Coming from #458 I agree that a generic tag implementation seems like the right idea. Before I spend too much time proposing, does the group have strong opinions on the two high level concepts:
I'm much less certain about the pitfalls in option 2, but these are the first front runners for a cross cutting implementation in my mind. I am still new to typescript and if there is a better pattern for this cross-cutting implementation throw some links my way and I'll read up. If we think one of these is a good starting point, I can dig a little deeper and create some code to prototype. Also for the sake of discussion the inheritance tree is the following AFAIK for |
Ok - big issue with option 1
Things like This raises a key question should |
In general, the CDK prefers composition over inheritance. So the rule of thumb is that constructs almost always directly extend the So for example, you might envision something like: interface ITaggable {
tags: Tags
}
class Tags {
addTag()
removeTag()
...
} And then, class VpcSubnet extends Construct implements ITaggable {
constructor(...) {
const resource = new cloudformation.VpcSubnetResource(this, 'Resource', ...);
this.tags = new Tags(resource); // or some other way to let the Tags class manage tags for the resource
}
}
|
Can we first list some use cases? I'm not entirely sure what users will expect from this functionality... |
@rix0rrr, you are right. Here's an attempt:
When aspects will be available, it should be possible to define aspects (#282) which will tag resources throughout a stack or a part of the stack. There is nothing special to do here in order to support this, but just calling it out. |
I see two tagging related gaps
|
I think tag propagation should be an option for each tag with a default=true. This matches other areas of the platform (such as auto scaling group tag propagation). |
Okay, but there was also some Kubernetes-related use case right? That's how this started. My question is not "how does the CDK enable tags", my question is "what do people use tags for".
|
👍 for A note about a default "name" tag: I think this is something we will be able to implement using aspects once we have a common tagging interface. The aspect will simply traverse the construct tree and will automatically assign a "Name" tag to all taggable constructs. I would argue that the default "Name" should be the construct's path (which is unique within the stack) and not the construct's id (which is only locally unique within it's parent construct). |
EKS has special tags that denote a VPC subnet as an EKS
I actually don't know the absolute answer here. For most things in EC2 console I know the |
This is where my question was really going. In order to do this I need to make I think we are in the right discussion, but if I sound way off the mark let me know. There is plenty of context I could be missing. |
I don’t think Construct needs to be aware of Resource in order to implement tag propagation. As mentioned above, Resource is only used as a base class for generated CloudFormation resources, and this is about tagging support at the AWS Construct Library layer, which a rich, object-oriented framework. AWS Constructs that support tagging should implement an interface I do realize there might be missing some design/architectural context, and happy to elaborate as needed. |
I think this might be my point of confusion. |
@moofish32 of course, the AWS Constructs will have to assign the tags to the resources they define. Something like #458 is definitely needed, but eventually we will want a uniform interface for tagging so we can later on do some cool "aspect-oriented" things with it. I am totally fine with #458 as-is until we have the tagging pattern established, or if you want to take a whack at a more general purpose programming model for tagging, you are more than welcome. Here's a sketch of my mental model around tags: interface ITaggable {
tags: Tags
};
// tag manager
class Tags {
private readonly tags: { [key: string]: string };
constructor(readonly parent: Construct, initialTags?: { [key: string]: string }) {
this.tags = initialTags || { }
}
toJson() {
return Object.keys(this.tags).map(key => ({ key, value: this.tags[key] }));
}
setTag(key: string, value: string, propagate = true) {
this.tags[key] = value;
if (propagate) {
applyTagToChildren(this.parent);
}
function applyTagToChildren(construct: Construct) {
for (const child of construct.children) {
if ('tags' in child) {
// child implements ITaggable (duck-typing)
child.tags.setTag(key, value);
}
applyTagToChildren(child); // recurse
}
}
}
}
// s3 bucket
interface BucketProps {
// ...
tags?: { [key: string]: string }
}
class Bucket extends Construct implements ITaggable {
public readonly tags: Tags;
constructor(...) {
this.tags = new Tags(this, props.tags);
new cloudformation.BucketResource(this, 'Resource', {
// ..
tags: new Token(() => this.tags.toJson()) // render tags for this resource during synthesis
});
}
} Now, I can do:
|
@moofish32 edited my comment above after reading your notes again... |
💡 the moment where academically I can describe composition over inheritance, turns out to have occurred much sooner than I could apply it. This was my confusion. I was trying to figure out how to put the TagManager trait on either |
Fixes aws#91, Closes aws#458 (as obsolete) The TagManager is a class Construct authors can use to implement tagging consistently. The manager provides the ability to propagate tags from parents to child, override parent tags in the child, set default tags on the child that can be overwritten by parents, and block tags from parents. Adding tagging support for the Vpc and Subnet constructs.
Fixes aws#91, Closes aws#458 (as obsolete) The TagManager is a class Construct authors can use to implement tagging consistently. The manager provides the ability to propagate tags from parents to child, override parent tags in the child, set default tags on the child that can be overwritten by parents, and block tags from parents. Adding tagging support for the Vpc and Subnet constructs.
* chore: add data structures for service connect config * chore(spec): remove erroneous LogConfiguration property * chore: fix signature of aliases
It should be possible to apply tags to AWS resources which support them and we would like to have a uniform API for tags across the AWS construct library
The text was updated successfully, but these errors were encountered: