Skip to content
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

Consistent Tags resource handling #63

Open
jaypipes opened this issue Jun 23, 2020 · 8 comments
Open

Consistent Tags resource handling #63

jaypipes opened this issue Jun 23, 2020 · 8 comments
Assignees
Labels
kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@jaypipes
Copy link
Collaborator

Different AWS service API use different data types to represent pretty much the exact same thing: a set of Key:Value pairs ("tags") for the resource. For instance, some APIs use a map[string]string as the underlying data type in API payloads. Some use a []struct{string, string}. Some use map[string]struct{}`, etc etc.

In addition to inconsistent data type representation of tags, the APIs also have inconsistent methods of adding, removing, replacing, and querying for tags on a resource. For example, some APIs have a TagResource and UntagResource API call. Others have an AddTag or RemoveTag call. Some APIs allow tagging a resource at create time. Others only allow updating tags after creation.

We want to make the experience of tagging and untagging resources consistent across all AWS APIs -- both from a data type representation as well as the behaviour of set/unset methods.

For all CRDs exposed by an ACK service controller, we want to have a consistent Spec.Tags field:

type {Resource}Spec struct {
    // Tags are a collection of string key/value pairs indicating the AWS
    // Tags that should be associated with the resource
    Tags map[string]string
    // Rest of Spec fields...
}

There will, of course, be no TagResource or UntagResource Kubernetes API call, since Kubernetes API is declarative and any changes to the desired state of a resource are simply handled by kubectl apply'ing the new desired resource state (as the CR's Spec struct).

So, the ACK service controller will need to have logic embedded in it that essentially instructs the service controller to handle tag information set/unset/add/remove logic in the way that the AWS service API for that controller expects. So, if the AWS service API has a TagResources API call and the only thing about a resource's desired state that has changed is the tags collection, then the service controller would call the TagResources API call, etc.

@jaypipes jaypipes added the kind/enhancement Categorizes issue or PR as related to existing feature enhancements. label Jun 23, 2020
@mhausenblas
Copy link
Contributor

Makes sense, yes. Came to a somewhat similar conclusion when I looked into the tagging across AWS services topic.

@nithu0115
Copy link
Contributor

nithu0115 commented Jun 23, 2020

@jaypipes, Make sense to use TagResources API call. Just a comment, TagResources API call does require additional permissions apart from service permissions which we need to document:

tag:GetResources
tag:TagResources
tag:UntagResources
tag:GetTagKeys
tag:GetTagValues

@mhausenblas
Copy link
Contributor

We should make sure it's aligned with AWS Resource Groups.

@ack-bot
Copy link
Collaborator

ack-bot commented Oct 20, 2021

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

@ack-bot ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 20, 2021
@a-hilaly
Copy link
Member

/lifecycle frozen

@ack-bot ack-bot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 21, 2021
@vijtrip2 vijtrip2 unpinned this issue Jan 28, 2022
@vijtrip2 vijtrip2 self-assigned this Apr 22, 2022
@haarchri
Copy link

any updates here ?

@jaypipes
Copy link
Collaborator Author

any updates here ?

Hi @haarchri! Yes, we're making progress in this effort. The first part of the progress is here:

aws-controllers-k8s/runtime#91
aws-controllers-k8s/runtime#90
aws-controllers-k8s/runtime#89
aws-controllers-k8s/runtime#88
aws-controllers-k8s/runtime#87
aws-controllers-k8s/code-generator#335

We're slowly defining what next steps are for standardizing existing controller's Tag representations.

@vijtrip2
Copy link
Contributor

Hi @haarchri , this work will be unblocked once we have multi version support. #835

Since some ACK controllers are GA now, we do not want to change the CRD's tag type (for consistent tag representation) without support for multi version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Categorizes issue or PR as related to existing feature enhancements. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
No open projects
Status: No status
Development

No branches or pull requests

8 participants