Merge pull request #10832 from rifelpet/aws-sdk

Add Tagging to Instance Profiles and OIDC Providers

cloudmock/aws/mockiam/iaminstanceprofile.go

@@ -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,
@@ -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,
 	}
 

cloudmock/aws/mockiam/iamrole.go

@@ -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,

cloudmock/aws/mockiam/iamrolepolicy.go

@@ -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")

cloudmock/aws/mockiam/oidcprovider.go

@@ -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,
 	}
@@ -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,
 	}

k8s/crds/kops.k8s.io_clusters.yaml

@@ -509,7 +509,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)

k8s/crds/kops.k8s.io_instancegroups.yaml

@@ -94,8 +94,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

pkg/apis/kops/cluster.go

@@ -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"`

pkg/apis/kops/instancegroup.go

@@ -133,7 +133,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"`

pkg/apis/kops/v1alpha2/cluster.go

@@ -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"`

pkg/apis/kops/v1alpha2/instancegroup.go

@@ -130,7 +130,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"`

pkg/model/awsmodel/oidc_provider.go

@@ -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,
 	})
 

pkg/model/iam.go

@@ -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)
 		}

pkg/resources/aws/aws.go

@@ -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
 //}
@@ -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
 			}
@@ -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
 			}
@@ -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
 	{
@@ -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)
 		}
 	}
 
@@ -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
 			}

upup/pkg/fi/cloudup/awstasks/iaminstanceprofile.go

@@ -35,6 +35,8 @@ type IAMInstanceProfile struct {
 	Name      *string
 	Lifecycle *fi.Lifecycle
 
+	Tags map[string]string
+
 	ID     *string
 	Shared *bool
 }
@@ -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
 		}
 	}
@@ -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
@@ -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)
@@ -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?

upup/pkg/fi/cloudup/awstasks/iaminstanceprofilerole.go

@@ -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
 		}
 	}
@@ -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 {

upup/pkg/fi/cloudup/awstasks/iamoidcprovider.go

@@ -39,6 +39,7 @@ type IAMOIDCProvider struct {
 	URL         *string
 
 	Name *string
+	Tags map[string]string
 
 	arn *string
 }
@@ -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,
 			}
 
@@ -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)
@@ -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
 }

upup/pkg/fi/cloudup/awstasks/iamrole.go

@@ -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
 		}
 	}

upup/pkg/fi/cloudup/awstasks/iamrolepolicy.go

@@ -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
 			}
 
@@ -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
 		}
 	}
@@ -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