Skip to content
This repository was archived by the owner on Jun 11, 2025. It is now read-only.

feat(apps/infra): fixes Check AWS Access implementation #261

Merged
nxtcoder17 merged 1 commit into from
Feb 8, 2024
Merged

feat(apps/infra): fixes Check AWS Access implementation #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .tools/nvim/__http__/infra/cloud-provider-secrets.graphql.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ global:
clusterName: sample-cluster

providerNamespace: kl-account-kloudlite-dev
providerName: aws-creds
providerName: aws-creds2

---

Expand Down
145 changes: 0 additions & 145 deletions apps/infra/internal/domain/provider-secrets-validator.go
View file
Open in desktop

This file was deleted.

49 changes: 42 additions & 7 deletions apps/infra/internal/domain/provider-secrets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
"bytes"
"context"
"fmt"
"strings"
"time"

fc "github.com/kloudlite/api/apps/infra/internal/entities/field-constants"
"github.com/kloudlite/api/common/fields"
"github.com/kloudlite/api/pkg/errors"
"github.com/kloudlite/operator/pkg/constants"
corev1 "k8s.io/api/core/v1"
"strings"
"time"

iamT "github.com/kloudlite/api/apps/iam/types"
"github.com/kloudlite/api/common"
Expand All @@ -22,7 +23,9 @@ import (
"github.com/kloudlite/api/pkg/repos"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/cloudformation"
"github.com/aws/aws-sdk-go/service/sts"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand All @@ -49,13 +52,15 @@ func generateAWSCloudformationTemplateUrl(args entities.AWSSecretCredentials, ev
return result.String(), nil
}

func (d *domain) validateAWSAssumeRole(_ context.Context, paramExternalId string, roleARN string) error {
func (d *domain) validateAWSAssumeRole(_ context.Context, paramExternalId string, roleARN string, instanceProfileName string, cfStackName string) error {
sess, err := session.NewSession()
sess.Config.Region = aws.String("ap-south-1")
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Feb 8, 2024

Choose a reason for hiding this comment

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

suggestion (llm): Hardcoding the AWS region to 'ap-south-1' may limit the flexibility of this function. Consider parameterizing the region or retrieving it from a configuration setting to enhance the function's adaptability to different deployment environments.

if err != nil {
d.logger.Errorf(err, "while creating new session")
return errors.NewE(err)
}

// 1. validating IAM Assume Role
svc := sts.New(sess)

resp, err := svc.AssumeRole(&sts.AssumeRoleInput{
Expand All @@ -68,8 +73,39 @@ func (d *domain) validateAWSAssumeRole(_ context.Context, paramExternalId string
return errors.NewE(err)
}

if resp.AssumedRoleUser.Arn != nil {
return nil
if resp.AssumedRoleUser == nil || resp.AssumedRoleUser.Arn == nil {
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Feb 8, 2024

Choose a reason for hiding this comment

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

suggestion (llm): The error message for a missing AWS assume role does not specify whether the role or the ARN is missing. Providing a more detailed error message could help with debugging.

return errors.Newf("AWS assume role (%s) not found", roleARN)
}

nsess, err := session.NewSession(&aws.Config{
Region: aws.String("ap-south-1"),
Credentials: credentials.NewStaticCredentials(*resp.Credentials.AccessKeyId, *resp.Credentials.SecretAccessKey, *resp.Credentials.SessionToken),
})
if err != nil {
return errors.NewE(err)
}

cf := cloudformation.New(nsess)
dso, err := cf.DescribeStacks(&cloudformation.DescribeStacksInput{
StackName: &cfStackName,
})
if err != nil {
return errors.NewE(err)
}

stackFound := false

for i := range dso.Stacks {
if dso.Stacks[i] != nil && *dso.Stacks[i].StackName == cfStackName {
stackFound = true
if *dso.Stacks[i].StackStatus != cloudformation.StackStatusCreateComplete {
return errors.Newf("cloudformation stack (%s) is not completed, yet", cfStackName)
}
}
}

if !stackFound {
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Feb 8, 2024

Choose a reason for hiding this comment

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

suggestion (llm): The error message 'waiting for cloudformation stack to be created' might be misleading as it suggests an action (waiting) rather than the actual error state (stack not found). Consider rephrasing to clearly indicate that the stack was not found.

return errors.Newf("waiting for cloudformation stack to be created")
}

return nil
Expand All @@ -94,7 +130,7 @@ func (d *domain) ValidateProviderSecretAWSAccess(ctx InfraContext, name string)
return nil, errors.NewE(err)
}

if err := d.validateAWSAssumeRole(ctx, psecret.AWS.CfParamExternalID, psecret.AWS.GetAssumeRoleRoleARN()); err != nil {
if err := d.validateAWSAssumeRole(ctx, psecret.AWS.CfParamExternalID, psecret.AWS.GetAssumeRoleRoleARN(), psecret.AWS.CfParamInstanceProfileName, psecret.AWS.CfParamStackName); err != nil {
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Feb 8, 2024

Choose a reason for hiding this comment

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

suggestion (llm): Adding parameters 'instanceProfileName' and 'cfStackName' to 'validateAWSAssumeRole' significantly increases the function's responsibilities. Consider refactoring to maintain single responsibility principle, possibly by splitting the validation of the assume role and the cloud formation stack into separate functions.

installationURL, err := generateAWSCloudformationTemplateUrl(*psecret.AWS, d.env)
if err != nil {
return nil, errors.NewE(err)
Expand Down Expand Up @@ -246,7 +282,6 @@ func (d *domain) UpdateProviderSecret(ctx InfraContext, providerSecretIn entitie
},
patchForUpdate,
)

if err != nil {
return nil, errors.NewE(err)
}
Expand Down