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

capi-add-tags-to-resources #170

Merged
merged 12 commits into from
Nov 6, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Add `global.podSecurityStandards.enforced` value for PSS migration.
- Add AWS tags to all created resources for CAPA and EKS clusters.

## [0.20.0] - 2023-09-21

Expand Down
11 changes: 7 additions & 4 deletions pkg/aws/services/acm/acm.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ func (s *Service) EnsureCertificate(domain string, customerTags map[string]strin
Key: aws.String(key.S3TagOrganization),
Value: aws.String(util.RemoveOrg(s.scope.ClusterNamespace())),
},
{
Key: aws.String(key.S3TagCluster),
Value: aws.String(s.scope.ClusterName()),
},
{
Key: aws.String(fmt.Sprintf(key.S3TagCloudProvider, s.scope.ClusterName())),
Value: aws.String("owned"),
Expand All @@ -51,6 +47,13 @@ func (s *Service) EnsureCertificate(domain string, customerTags map[string]strin
},
ValidationMethod: aws.String(acm.ValidationMethodDns),
}
// add cluster tag if missing (this is case for vintage clusters)
if _, ok := customerTags[key.S3TagCluster]; !ok {
if customerTags == nil {
customerTags = make(map[string]string)
}
customerTags[key.S3TagCluster] = s.scope.ClusterName()
}

for k, v := range customerTags {
tag := &acm.Tag{
Expand Down
13 changes: 11 additions & 2 deletions pkg/aws/services/cloudfront/cloudfront.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ func (s *Service) EnsureDistribution(config DistributionConfig) (*Distribution,

// Add internal and customer tags.
{
// deep copy the map from config.CustomerTags
customerTags := make(map[string]string)
for k, v := range config.CustomerTags {
customerTags[k] = v
}
for k, v := range s.internalTags() {
tag := &cloudfront.Tag{
Key: aws.String(k),
Expand All @@ -135,7 +140,12 @@ func (s *Service) EnsureDistribution(config DistributionConfig) (*Distribution,
i.DistributionConfigWithTags.Tags.Items = append(i.DistributionConfigWithTags.Tags.Items, tag)
}

for k, v := range config.CustomerTags {
// add cluster tag if missing (this is case for vintage clusters)
if _, ok := customerTags[key.S3TagCluster]; !ok {
customerTags[key.S3TagCluster] = s.scope.ClusterName()
}

for k, v := range customerTags {
tag := &cloudfront.Tag{
Key: aws.String(k),
Value: aws.String(v),
Expand Down Expand Up @@ -268,7 +278,6 @@ func (s *Service) findDistribution() (*Distribution, error) {
func (s *Service) internalTags() map[string]string {
return map[string]string{
key.S3TagOrganization: util.RemoveOrg(s.scope.ClusterNamespace()),
key.S3TagCluster: s.scope.ClusterName(),
fmt.Sprintf(key.S3TagCloudProvider, s.scope.ClusterName()): "owned",
key.S3TagInstallation: s.scope.Installation(),
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/aws/services/s3/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ func (s *Service) CreateTags(bucketName string, customerTags map[string]string)
Key: aws.String(key.S3TagOrganization),
Value: aws.String(util.RemoveOrg(s.scope.ClusterNamespace())),
},
{
Key: aws.String(key.S3TagCluster),
Value: aws.String(s.scope.ClusterName()),
},
{
Key: aws.String(fmt.Sprintf(key.S3TagCloudProvider, s.scope.ClusterName())),
Value: aws.String("owned"),
Expand All @@ -65,6 +61,13 @@ func (s *Service) CreateTags(bucketName string, customerTags map[string]string)
},
},
}
// add cluster tag if missing (this is case for vintage clusters)
if _, ok := customerTags[key.S3TagCluster]; !ok {
if customerTags == nil {
customerTags = make(map[string]string)
}
customerTags[key.S3TagCluster] = s.scope.ClusterName()
}

for k, v := range customerTags {
i.Tagging.TagSet = append(i.Tagging.TagSet, &s3.Tag{Key: aws.String(k), Value: aws.String(v)})
Expand Down
22 changes: 9 additions & 13 deletions pkg/irsa/capa/capa.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
capi "sigs.k8s.io/cluster-api/api/v1beta1"
capa "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/giantswarm/irsa-operator/pkg/aws/scope"
Expand Down Expand Up @@ -95,17 +95,13 @@ func (s *Service) Reconcile(ctx context.Context) error {
return err
}

// Fetch custom tags from Cluster CR
cluster := &capi.Cluster{}
err = s.Client.Get(ctx, types.NamespacedName{Namespace: s.Scope.ClusterNamespace(), Name: s.Scope.ClusterName()}, cluster)
if apierrors.IsNotFound(err) {
// fallthrough
} else if err != nil {
// Fetch custom tags from AWSCluster CR
awsCluster := &capa.AWSCluster{}
err = s.Client.Get(ctx, types.NamespacedName{Namespace: s.Scope.ClusterNamespace(), Name: s.Scope.ClusterName()}, awsCluster)
if err != nil {
return err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we specifically handle apierrors.IsNotFound(err), as before?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea, yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think its a good idea to continue if the AWSCluster CR is missing, because in that case either something terrible happened or it's some kind of super early in the creation processes and it would mean we ignore the tags that are supposed to be there.

this code feels like it was copied from somewhere where it make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it on purpose

}
customerTags := key.GetCustomerTags(cluster)

err = s.S3.CreateTags(s.Scope.BucketName(), customerTags)
err = s.S3.CreateTags(s.Scope.BucketName(), awsCluster.Spec.AdditionalTags)
if err != nil {
ctrlmetrics.Errors.WithLabelValues(s.Scope.Installation(), s.Scope.AccountID(), s.Scope.ClusterName(), s.Scope.ClusterNamespace()).Inc()
s.Scope.Logger().Error(err, "failed to create tags")
Expand All @@ -122,7 +118,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
cloudfrontAliasDomain := s.getCloudFrontAliasDomain()
if cloudfrontAliasDomain != "" {
// Ensure ACM certificate.
certificateArn, err := s.ACM.EnsureCertificate(cloudfrontAliasDomain, customerTags)
certificateArn, err := s.ACM.EnsureCertificate(cloudfrontAliasDomain, awsCluster.Spec.AdditionalTags)
if err != nil {
ctrlmetrics.Errors.WithLabelValues(s.Scope.Installation(), s.Scope.AccountID(), s.Scope.ClusterName(), s.Scope.ClusterNamespace()).Inc()
s.Scope.Logger().Error(err, "failed to create ACM certificate")
Expand Down Expand Up @@ -180,7 +176,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
cloudfrontCertificateARN = *certificateArn
}

distribution, err = s.Cloudfront.EnsureDistribution(cloudfront.DistributionConfig{CustomerTags: customerTags, Aliases: aliases, CertificateArn: cloudfrontCertificateARN})
distribution, err = s.Cloudfront.EnsureDistribution(cloudfront.DistributionConfig{CustomerTags: awsCluster.Spec.AdditionalTags, Aliases: aliases, CertificateArn: cloudfrontCertificateARN})
if err != nil {
ctrlmetrics.Errors.WithLabelValues(s.Scope.Installation(), s.Scope.AccountID(), s.Scope.ClusterName(), s.Scope.ClusterNamespace()).Inc()
s.Scope.Logger().Error(err, "failed to create cloudfront distribution")
Expand Down Expand Up @@ -313,7 +309,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
identityProviderURLs = append(identityProviderURLs, util.EnsureHTTPS(*alias))
}

return s.IAM.EnsureOIDCProviders(identityProviderURLs, key.STSUrl(s.Scope.Region()), customerTags)
return s.IAM.EnsureOIDCProviders(identityProviderURLs, key.STSUrl(s.Scope.Region()), awsCluster.Spec.AdditionalTags)
}
err = backoff.Retry(createOIDCProvider, b)
if err != nil {
Expand Down
9 changes: 4 additions & 5 deletions pkg/irsa/eks/eks.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (

"github.com/giantswarm/microerror"
"k8s.io/apimachinery/pkg/types"
capi "sigs.k8s.io/cluster-api/api/v1beta1"
controlplanecapa "sigs.k8s.io/cluster-api-provider-aws/v2/controlplane/eks/api/v1beta2"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/giantswarm/irsa-operator/pkg/aws/scope"
Expand Down Expand Up @@ -41,15 +41,14 @@ func (s *Service) Reconcile(ctx context.Context) error {
}
identityProviderURLs := []string{oidcURL}

// Fetch custom tags from Cluster CR
cluster := &capi.Cluster{}
// Fetch custom tags from AWSManagedControlPlane CR
cluster := &controlplanecapa.AWSManagedControlPlane{}
err = s.Client.Get(ctx, types.NamespacedName{Namespace: s.Scope.ClusterNamespace(), Name: s.Scope.ClusterName()}, cluster)
if err != nil {
return microerror.Mask(err)
}
customerTags := key.GetCustomerTags(cluster)

err = s.IAM.EnsureOIDCProviders(identityProviderURLs, key.STSUrl(s.Scope.Region()), customerTags)
err = s.IAM.EnsureOIDCProviders(identityProviderURLs, key.STSUrl(s.Scope.Region()), cluster.Spec.AdditionalTags)
if err != nil {
ctrlmetrics.Errors.WithLabelValues(s.Scope.Installation(), s.Scope.AccountID(), s.Scope.ClusterName(), s.Scope.ClusterNamespace()).Inc()
s.Scope.Logger().Error(err, "failed to create OIDC provider")
Expand Down