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

Add Tagging to Instance Profiles and OIDC Providers #10832

Merged
merged 3 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cloudmock/aws/mockiam/iaminstanceprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (m *MockIAM) GetInstanceProfile(request *iam.GetInstanceProfileInput) (*iam

ip := m.InstanceProfiles[aws.StringValue(request.InstanceProfileName)]
if ip == nil {
return nil, awserr.New("NoSuchEntity", "No such entity", nil)
return nil, awserr.New(iam.ErrCodeNoSuchEntityException, "No such entity", nil)
}
response := &iam.GetInstanceProfileOutput{
InstanceProfile: ip,
Expand All @@ -57,6 +57,7 @@ func (m *MockIAM) CreateInstanceProfile(request *iam.CreateInstanceProfileInput)
// Arn: request.Arn,
// InstanceProfileId: request.InstanceProfileId,
Path: request.Path,
Tags: request.Tags,
// Roles: request.Roles,
}

Expand Down
2 changes: 1 addition & 1 deletion cloudmock/aws/mockiam/iamrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (m *MockIAM) GetRole(request *iam.GetRoleInput) (*iam.GetRoleOutput, error)

role := m.Roles[aws.StringValue(request.RoleName)]
if role == nil {
return nil, awserr.New("NoSuchEntity", "No such entity", nil)
return nil, awserr.New(iam.ErrCodeNoSuchEntityException, "No such entity", nil)
}
response := &iam.GetRoleOutput{
Role: role,
Expand Down
2 changes: 1 addition & 1 deletion cloudmock/aws/mockiam/iamrolepolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (m *MockIAM) GetRolePolicy(request *iam.GetRolePolicyInput) (*iam.GetRolePo
}
return response, nil
}
return nil, awserr.New("NoSuchEntity", "No such entity", nil)
return nil, awserr.New(iam.ErrCodeNoSuchEntityException, "No such entity", nil)
}
func (m *MockIAM) GetRolePolicyWithContext(aws.Context, *iam.GetRolePolicyInput, ...request.Option) (*iam.GetRolePolicyOutput, error) {
panic("Not implemented")
Expand Down
2 changes: 2 additions & 0 deletions cloudmock/aws/mockiam/oidcprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func (m *MockIAM) GetOpenIDConnectProviderWithContext(ctx aws.Context, request *
response := &iam.GetOpenIDConnectProviderOutput{
ClientIDList: provider.ClientIDList,
CreateDate: provider.CreateDate,
Tags: provider.Tags,
ThumbprintList: provider.ThumbprintList,
Url: provider.Url,
}
Expand All @@ -87,6 +88,7 @@ func (m *MockIAM) CreateOpenIDConnectProvider(request *iam.CreateOpenIDConnectPr

p := &iam.GetOpenIDConnectProviderOutput{
ClientIDList: request.ClientIDList,
Tags: request.Tags,
ThumbprintList: request.ThumbprintList,
Url: request.Url,
}
Expand Down
3 changes: 2 additions & 1 deletion k8s/crds/kops.k8s.io_clusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,8 @@ spec:
cloudLabels:
additionalProperties:
type: string
description: Tags for AWS resources
description: CloudLabels defines additional tags or labels on cloud
provider resources
type: object
cloudProvider:
description: The CloudProvider to use (aws or gce)
Expand Down
4 changes: 2 additions & 2 deletions k8s/crds/kops.k8s.io_instancegroups.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ spec:
cloudLabels:
additionalProperties:
type: string
description: CloudLabels indicates the labels for instances in this
group, at the AWS level
description: CloudLabels defines additional tags or labels on cloud
provider resources
type: object
compressUserData:
description: CompressUserData compresses parts of the user data to
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ type ClusterSpec struct {
Authorization *AuthorizationSpec `json:"authorization,omitempty"`
// NodeAuthorization defined the custom node authorization configuration
NodeAuthorization *NodeAuthorizationSpec `json:"nodeAuthorization,omitempty"`
// Tags for AWS instance groups
// CloudLabels defines additional tags or labels on cloud provider resources
CloudLabels map[string]string `json:"cloudLabels,omitempty"`
// Hooks for custom actions e.g. on first installation
Hooks []HookSpec `json:"hooks,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ type InstanceGroupSpec struct {
AssociatePublicIP *bool `json:"associatePublicIp,omitempty"`
// AdditionalSecurityGroups attaches additional security groups (e.g. i-123456)
AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
// CloudLabels indicates the labels for instances in this group, at the AWS level
// CloudLabels defines additional tags or labels on cloud provider resources
CloudLabels map[string]string `json:"cloudLabels,omitempty"`
// NodeLabels indicates the kubernetes labels for nodes in this group
NodeLabels map[string]string `json:"nodeLabels,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/v1alpha2/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ type ClusterSpec struct {
Authorization *AuthorizationSpec `json:"authorization,omitempty"`
// NodeAuthorization defined the custom node authorization configuration
NodeAuthorization *NodeAuthorizationSpec `json:"nodeAuthorization,omitempty"`
// Tags for AWS resources
// CloudLabels defines additional tags or labels on cloud provider resources
CloudLabels map[string]string `json:"cloudLabels,omitempty"`
// Hooks for custom actions e.g. on first installation
Hooks []HookSpec `json:"hooks,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/v1alpha2/instancegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ type InstanceGroupSpec struct {
AssociatePublicIP *bool `json:"associatePublicIp,omitempty"`
// AdditionalSecurityGroups attaches additional security groups (e.g. i-123456)
AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
// CloudLabels indicates the labels for instances in this group, at the AWS level
// CloudLabels defines additional tags or labels on cloud provider resources
CloudLabels map[string]string `json:"cloudLabels,omitempty"`
// NodeLabels indicates the kubernetes labels for nodes in this group
NodeLabels map[string]string `json:"nodeLabels,omitempty"`
Expand Down
1 change: 1 addition & 0 deletions pkg/model/awsmodel/oidc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (b *OIDCProviderBuilder) Build(c *fi.ModelBuilderContext) error {
Lifecycle: b.Lifecycle,
URL: fi.String(issuerURL),
ClientIDs: []*string{fi.String(defaultAudience)},
Tags: b.CloudTags(b.ClusterName(), false),
Thumbprints: thumbprints,
})

Expand Down
1 change: 1 addition & 0 deletions pkg/model/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ func (b *IAMModelBuilder) buildIAMTasks(role iam.Subject, iamName string, c *fi.
Name: s(iamName),
Lifecycle: b.Lifecycle,
Shared: fi.Bool(shared),
Tags: b.CloudTags(iamName, false),
}
c.AddTask(iamInstanceProfile)
}
Expand Down
39 changes: 27 additions & 12 deletions pkg/resources/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,24 @@ func matchesElbV2Tags(tags map[string]string, actual []*elbv2.Tag) bool {
return true
}

func matchesIAMTags(tags map[string]string, actual []*iam.Tag) bool {
for k, v := range tags {
found := false
for _, a := range actual {
if aws.StringValue(a.Key) == k {
if aws.StringValue(a.Value) == v {
found = true
break
}
}
}
if !found {
return false
}
}
return true
}

//type DeletableResource interface {
// Delete(cloud fi.Cloud) error
//}
Expand Down Expand Up @@ -1906,7 +1924,7 @@ func DeleteIAMRole(cloud fi.Cloud, r *resources.Resource) error {
return true
})
if err != nil {
if awsup.AWSErrorCode(err) == "NoSuchEntity" {
if awsup.AWSErrorCode(err) == iam.ErrCodeNoSuchEntityException {
klog.V(2).Infof("Got NoSuchEntity describing IAM RolePolicy %q; will treat as already-deleted", roleName)
return nil
}
Expand All @@ -1925,7 +1943,7 @@ func DeleteIAMRole(cloud fi.Cloud, r *resources.Resource) error {
return true
})
if err != nil {
if awsup.AWSErrorCode(err) == "NoSuchEntity" {
if awsup.AWSErrorCode(err) == iam.ErrCodeNoSuchEntityException {
klog.V(2).Infof("Got NoSuchEntity describing IAM RolePolicy %q; will treat as already-detached", roleName)
return nil
}
Expand Down Expand Up @@ -2097,6 +2115,7 @@ func ListIAMInstanceProfiles(cloud fi.Cloud, clusterName string) ([]*resources.R

func ListIAMOIDCProviders(cloud fi.Cloud, clusterName string) ([]*resources.Resource, error) {
c := cloud.(awsup.AWSCloud)
tags := c.Tags()

var providers []*string
{
Expand All @@ -2110,18 +2129,14 @@ func ListIAMOIDCProviders(cloud fi.Cloud, clusterName string) ([]*resources.Reso
descReq := &iam.GetOpenIDConnectProviderInput{
OpenIDConnectProviderArn: arn,
}
_, err := c.IAM().GetOpenIDConnectProvider(descReq)
resp, err := c.IAM().GetOpenIDConnectProvider(descReq)
if err != nil {
return nil, fmt.Errorf("error getting IAM OIDC Provider: %v", err)
}
// TODO: only delete oidc providers if they're owned by the cluster.
// We need to figure out how we can determine that given only a cluster name.
// Providers dont support tagging or naming.

// providers = append(providers, arn)
}
if err != nil {
return nil, fmt.Errorf("error listing IAM roles: %v", err)
if !matchesIAMTags(tags, resp.Tags) {
continue
}
providers = append(providers, arn)
}
}

Expand Down Expand Up @@ -2150,7 +2165,7 @@ func DeleteIAMOIDCProvider(cloud fi.Cloud, r *resources.Resource) error {
}
_, err := c.IAM().DeleteOpenIDConnectProvider(request)
if err != nil {
if awsup.AWSErrorCode(err) == "NoSuchEntity" {
if awsup.AWSErrorCode(err) == iam.ErrCodeNoSuchEntityException {
klog.V(2).Infof("Got NoSuchEntity deleting IAM OIDC Provider %v; will treat as already-deleted", arn)
return nil
}
Expand Down
33 changes: 32 additions & 1 deletion upup/pkg/fi/cloudup/awstasks/iaminstanceprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type IAMInstanceProfile struct {
Name *string
Lifecycle *fi.Lifecycle

Tags map[string]string

ID *string
Shared *bool
}
Expand All @@ -52,7 +54,7 @@ func findIAMInstanceProfile(cloud awsup.AWSCloud, name string) (*iam.InstancePro

response, err := cloud.IAM().GetInstanceProfile(request)
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NoSuchEntity" {
if awsErr.Code() == iam.ErrCodeNoSuchEntityException {
return nil, nil
}
}
Expand All @@ -79,6 +81,7 @@ func (e *IAMInstanceProfile) Find(c *fi.Context) (*IAMInstanceProfile, error) {
actual := &IAMInstanceProfile{
ID: p.InstanceProfileId,
Name: p.InstanceProfileName,
Tags: mapIAMTagsToMap(p.Tags),
}

e.ID = actual.ID
Expand Down Expand Up @@ -114,6 +117,7 @@ func (_ *IAMInstanceProfile) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAM

request := &iam.CreateInstanceProfileInput{
InstanceProfileName: e.Name,
Tags: mapToIAMTags(e.Tags),
}

response, err := t.Cloud.IAM().CreateInstanceProfile(request)
Expand All @@ -123,6 +127,33 @@ func (_ *IAMInstanceProfile) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAM

e.ID = response.InstanceProfile.InstanceProfileId
e.Name = response.InstanceProfile.InstanceProfileName
} else {
if changes.Tags != nil {
if len(a.Tags) > 0 {
existingTagKeys := make([]*string, 0)
for k := range a.Tags {
existingTagKeys = append(existingTagKeys, &k)
}
untagRequest := &iam.UntagInstanceProfileInput{
InstanceProfileName: a.Name,
TagKeys: existingTagKeys,
}
_, err := t.Cloud.IAM().UntagInstanceProfile(untagRequest)
if err != nil {
return fmt.Errorf("error untagging IAMInstanceProfile: %v", err)
}
}
if len(e.Tags) > 0 {
tagRequest := &iam.TagInstanceProfileInput{
InstanceProfileName: a.Name,
Tags: mapToIAMTags(e.Tags),
}
_, err := t.Cloud.IAM().TagInstanceProfile(tagRequest)
if err != nil {
return fmt.Errorf("error tagging IAMInstanceProfile: %v", err)
}
}
}
}

// TODO: Should we use path as our tag?
Expand Down
3 changes: 2 additions & 1 deletion upup/pkg/fi/cloudup/awstasks/iaminstanceprofilerole.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (e *IAMInstanceProfileRole) Find(c *fi.Context) (*IAMInstanceProfileRole, e

response, err := cloud.IAM().GetInstanceProfile(request)
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NoSuchEntity" {
if awsErr.Code() == iam.ErrCodeNoSuchEntityException {
return nil, nil
}
}
Expand Down Expand Up @@ -114,6 +114,7 @@ func (_ *IAMInstanceProfileRole) RenderAWS(t *awsup.AWSAPITarget, a, e, changes
type terraformIAMInstanceProfile struct {
Name *string `json:"name" cty:"name"`
Role *terraform.Literal `json:"role" cty:"role"`
// TODO(rifelpet): add tags field when terraform supports it
}

func (_ *IAMInstanceProfileRole) RenderTerraform(t *terraform.TerraformTarget, a, e, changes *IAMInstanceProfileRole) error {
Expand Down
29 changes: 29 additions & 0 deletions upup/pkg/fi/cloudup/awstasks/iamoidcprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type IAMOIDCProvider struct {
URL *string

Name *string
Tags map[string]string

arn *string
}
Expand Down Expand Up @@ -83,6 +84,7 @@ func (e *IAMOIDCProvider) Find(c *fi.Context) (*IAMOIDCProvider, error) {
ClientIDs: descResp.ClientIDList,
Thumbprints: actualThumbprints,
URL: &actualURL,
Tags: mapIAMTagsToMap(descResp.Tags),
arn: arn,
}

Expand Down Expand Up @@ -135,6 +137,7 @@ func (p *IAMOIDCProvider) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAMOID
ClientIDList: e.ClientIDs,
ThumbprintList: aws.StringSlice(thumbprints),
Url: e.URL,
Tags: mapToIAMTags(e.Tags),
}

response, err := t.Cloud.IAM().CreateOpenIDConnectProvider(request)
Expand All @@ -156,6 +159,32 @@ func (p *IAMOIDCProvider) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAMOID
return fmt.Errorf("error updating IAMOIDCProvider Thumbprints: %v", err)
}
}
if changes.Tags != nil {
if len(a.Tags) > 0 {
existingTagKeys := make([]*string, 0)
for k := range a.Tags {
existingTagKeys = append(existingTagKeys, &k)
}
untagRequest := &iam.UntagOpenIDConnectProviderInput{
OpenIDConnectProviderArn: a.arn,
TagKeys: existingTagKeys,
}
_, err = t.Cloud.IAM().UntagOpenIDConnectProvider(untagRequest)
if err != nil {
return fmt.Errorf("error untagging IAMOIDCProvider: %v", err)
}
}
if len(e.Tags) > 0 {
tagRequest := &iam.TagOpenIDConnectProviderInput{
OpenIDConnectProviderArn: a.arn,
Tags: mapToIAMTags(e.Tags),
}
_, err = t.Cloud.IAM().TagOpenIDConnectProvider(tagRequest)
if err != nil {
return fmt.Errorf("error tagging IAMOIDCProvider: %v", err)
}
}
}
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/awstasks/iamrole.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (e *IAMRole) Find(c *fi.Context) (*IAMRole, error) {

response, err := cloud.IAM().GetRole(request)
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NoSuchEntity" {
if awsErr.Code() == iam.ErrCodeNoSuchEntityException {
return nil, nil
}
}
Expand Down
6 changes: 3 additions & 3 deletions upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (e *IAMRolePolicy) Find(c *fi.Context) (*IAMRolePolicy, error) {

response, err := cloud.IAM().ListAttachedRolePolicies(request)
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NoSuchEntity" {
if awsErr.Code() == iam.ErrCodeNoSuchEntityException {
return nil, nil
}

Expand Down Expand Up @@ -94,7 +94,7 @@ func (e *IAMRolePolicy) Find(c *fi.Context) (*IAMRolePolicy, error) {

response, err := cloud.IAM().GetRolePolicy(request)
if awsErr, ok := err.(awserr.Error); ok {
if awsErr.Code() == "NoSuchEntity" {
if awsErr.Code() == iam.ErrCodeNoSuchEntityException {
return nil, nil
}
}
Expand Down Expand Up @@ -230,7 +230,7 @@ func (_ *IAMRolePolicy) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *IAMRoleP
klog.V(2).Infof("Deleting role policy %s/%s", aws.StringValue(e.Role.Name), aws.StringValue(e.Name))
_, err = t.Cloud.IAM().DeleteRolePolicy(request)
if err != nil {
if awsup.AWSErrorCode(err) == "NoSuchEntity" {
if awsup.AWSErrorCode(err) == iam.ErrCodeNoSuchEntityException {
// Already deleted
klog.V(2).Infof("Got NoSuchEntity deleting role policy %s/%s; assuming does not exist", aws.StringValue(e.Role.Name), aws.StringValue(e.Name))
return nil
Expand Down