-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
aws_auth_type = "iam_user" | ||
aws_iam_user = { | ||
access_key_id = "AKIAIOSFODNN7EXAMPLE" | ||
secret_access_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" |
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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!
- fixed 3072248
// S3 Fields | ||
AWSAuthType: types.StringPointerValue(connection.AWSAuthType), | ||
AWSIAMUser: plan.NewAWSIAMUser(), | ||
AWSAssumeRole: plan.NewAWSAssumeRole(), |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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()), | ||
} | ||
} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, thx! d4037ec
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
|
Summary
It adds a new connector type
s3
.Note
Changes