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 connection_type: S3 #40

Merged
merged 29 commits into from
Feb 11, 2025
Merged

add connection_type: S3 #40

merged 29 commits into from
Feb 11, 2025

Conversation

u110
Copy link
Collaborator

@u110 u110 commented Feb 3, 2025

Summary

It adds a new connector type s3.

Note

  • TROCCO's Web API for S3 is not yet publicly available, so do not merge right now.

Changes

  • Add connector_type: s3

@u110 u110 changed the title S3 connection add connection_type: S3 Feb 3, 2025
@u110 u110 marked this pull request as ready for review February 3, 2025 02:59
@u110 u110 requested a review from gtnao as a code owner February 3, 2025 02:59
Base automatically changed from mysql-connection to main February 3, 2025 04:36
aws_auth_type = "iam_user"
aws_iam_user = {
access_key_id = "AKIAIOSFODNN7EXAMPLE"
secret_access_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"
Copy link
Collaborator

@okeyaki okeyaki Feb 3, 2025

Choose a reason for hiding this comment

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

IMO: It seems the secret access key is a dummy. However, it would be troublesome to be detected tools like git-secrets, so the following description might work better:

secret_access_key = "YOUR_SECRET_ACCESS_KEY"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's seems to be better!

// S3 Fields
AWSAuthType: types.StringPointerValue(connection.AWSAuthType),
AWSIAMUser: plan.NewAWSIAMUser(),
AWSAssumeRole: plan.NewAWSAssumeRole(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

ASK: Cannot AWSIAMUser: plan.NewAWSIAMUser() be replaced with AWSIAMUser: plan.AWSIAMUser?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ran it with the suggested modifications and there seemed to be no problem.
so, there looked to be no need to COPY it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AssumeRole is included in the API results, so I modified it to refer to it.

AccessKeyID: types.StringPointerValue(m.AWSIAMUser.AccessKeyID.ValueStringPointer()),
SecretAccessKey: types.StringPointerValue(m.AWSIAMUser.SecretAccessKey.ValueStringPointer()),
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO: It seems to be a bit better to implement AWSIAMUser.Copy() instead of NewAWSIAMUser.

Copy link
Collaborator Author

@u110 u110 Feb 3, 2025

Choose a reason for hiding this comment

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

@u110 u110 requested a review from okeyaki February 4, 2025 00:59
@katamotokosuke
Copy link
Collaborator

resource "trocco_connection" "s3" {  
  name        = "S3 Example"  
  connection_type = "s3"  
  description = "This is a AWS S3 connection example"  
  
  aws_auth_type = "iam_user"  
  aws_iam_user = {  
    access_key_id     = "YOUR_ACCESS_KEY_ID"  
    secret_access_key = "YOUR_SECRET_ACCESS_KEY"  
  }  
  aws_assume_role = {  
    account_id = "s"  
    role_name = "role_name"  
  }  
}

If you specify aws_iam_user and aws_assume_role at the same time, there will always be a difference.

kosuke.katamoto@pN-mac-0001 terraform-test-for-trocco % terraform apply
trocco_connection.s3: Refreshing state... [name=S3 Example]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # trocco_connection.s3 will be updated in-place
  ~ resource "trocco_connection" "s3" {
      + aws_assume_role = {
          + account_id = "s"
          + role_name  = "role_name"
        }
        id              = 4
        name            = "S3 Example"
        # (4 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

trocco_connection.s3: Modifying... [name=S3 Example]
trocco_connection.s3: Modifications complete after 1s [name=S3 Example]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.
kosuke.katamoto@pN-mac-0001 terraform-test-for-trocco % terraform apply
trocco_connection.s3: Refreshing state... [name=S3 Example]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # trocco_connection.s3 will be updated in-place
  ~ resource "trocco_connection" "s3" {
      + aws_assume_role = {
          + account_id = "s"
          + role_name  = "role_name"
        }
        id              = 4
        name            = "S3 Example"
        # (4 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

trocco_connection.s3: Modifying... [name=S3 Example]
trocco_connection.s3: Modifications complete after 0s [name=S3 Example]

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

@u110
Copy link
Collaborator Author

u110 commented Feb 7, 2025

If you specify aws_iam_user and aws_assume_role at the same time, there will always be a difference.

I'll add validation process for not to use together aws_iam_user and aws_assume_role, thx!

@u110 u110 marked this pull request as draft February 7, 2025 01:04
@u110 u110 marked this pull request as ready for review February 7, 2025 01:49
@u110 u110 merged commit 41960d0 into main Feb 11, 2025
6 checks passed
@u110 u110 deleted the s3-connection branch February 11, 2025 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants