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.

Sign up for GitHub

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
Merged

capi-add-tags-to-resources #170

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b2eff2d
capi-add-tags-to-resources
calvix Nov 1, 2023
4f453c9
capi-add-tags-to-resources
calvix Nov 1, 2023
5f0de04
capi-add-tags-to-resources
calvix Nov 1, 2023
60e4bf8
fix-dsitribution
calvix Nov 1, 2023
5a0e4b6
fix-comment
calvix Nov 2, 2023
f241b63
Merge branch 'master' into capi-add-tags-to-resources
calvix Nov 2, 2023
c3e9491
whitespace
calvix Nov 2, 2023
42f5444
Merge branch 'capi-add-tags-to-resources' of github.com:giantswarm/ir…
calvix Nov 2, 2023
e98f6f0
deepcopy
calvix Nov 2, 2023
4f43d4f
fix
calvix Nov 2, 2023
bcb03f0
fix3
calvix Nov 2, 2023
cea6f6a
Merge branch 'master' into capi-add-tags-to-resources
calvix Nov 6, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
View file
Open in desktop
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
12 changes: 10 additions & 2 deletions pkg/aws/services/cloudfront/cloudfront.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ func (s *Service) EnsureDistribution(config DistributionConfig) (*Distribution,

// Add internal and customer tags.
{
customerTags := config.CustomerTags
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
customerTags := config.CustomerTags
customerTags := config.CustomerTags.DeepCopy()

so we don't mutate the input parameter which may lead to surprises

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DeepCopy function does not exists for map[], so doing the ugly copy via for

for k, v := range s.internalTags() {
tag := &cloudfront.Tag{
Key: aws.String(k),
Expand All @@ -135,7 +136,15 @@ 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 {
if customerTags == nil {
customerTags = make(map[string]string)
}
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 +277,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
13 changes: 8 additions & 5 deletions pkg/aws/services/s3/bucket.go
View file
Open in desktop
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 Expand Up @@ -136,6 +139,7 @@ func (s *Service) IsBucketReady(bucketName string) error {
}

func (s *Service) UpdatePolicy(bucketName, oaiId string) error {

calvix marked this conversation as resolved.
Show resolved Hide resolved
var cloudfrontPolicy = `{
"Version": "2012-10-17",
"Id": "PolicyForCloudFrontPrivateContent",
Expand All @@ -162,7 +166,6 @@ func (s *Service) UpdatePolicy(bucketName, oaiId string) error {
}
]
}`

t, err := template.New("").Parse(cloudfrontPolicy)
if err != nil {
return err
Expand Down
22 changes: 9 additions & 13 deletions pkg/irsa/capa/capa.go
View file
Open in desktop
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
View file
Open in desktop
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