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

✨ aws ec2 eips, vpc natgateways, vpc peering conn, service endpoints #3929

Merged
merged 2 commits into from
May 9, 2024

Conversation

vjeffrey
Copy link
Contributor

@vjeffrey vjeffrey commented May 7, 2024

No description provided.

@vjeffrey vjeffrey marked this pull request as draft May 7, 2024 00:55
Copy link
Contributor

github-actions bot commented May 7, 2024

Test Results

2 984 tests  ±0   2 983 ✅ ±0   1m 26s ⏱️ +2s
  329 suites ±0       1 💤 ±0 
   23 files   ±0       0 ❌ ±0 

Results for commit 7c47371. ± Comparison against base commit 3a2bb65.

♻️ This comment has been updated with latest results.

This comment has been minimized.

This comment has been minimized.

@vjeffrey vjeffrey marked this pull request as ready for review May 8, 2024 00:06
Copy link
Contributor

@czunker czunker left a comment

Choose a reason for hiding this comment

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

Nice list of resources

publicIp string
// false if allocationId not present
attached bool
// Ec2 instance associated with the EIP
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not that familiar with AWS. Can an EIP only be attached to an EC2 instance. Not something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems like it from looking at the api

providers/aws/resources/aws.lr.manifest.yaml Show resolved Hide resolved
providers/aws/resources/aws.lr.manifest.yaml Show resolved Hide resolved
@@ -102,6 +104,128 @@ func (a *mqlAws) getVpcs(conn *connection.AwsConnection) []*jobpool.Job {
return tasks
}

func (a *mqlAwsVpcNatgatewayAddress) id() (string, error) {
return a.AllocationId.Data, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Can a NAT GW IP not be assigned and this empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, yup, nice catch, modifying this

Copy link
Contributor

@czunker czunker left a comment

Choose a reason for hiding this comment

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

Thanks @vjeffrey

Copy link
Contributor

@misterpantz misterpantz left a comment

Choose a reason for hiding this comment

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

Hi VJ! Thank you for thoroughly documenting these new resources! Tim asked me to go through and polish the help. Please let me know if any of my suggestions don't make sense.

providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
providers/aws/resources/aws.lr Outdated Show resolved Hide resolved
@vjeffrey vjeffrey merged commit 4288779 into main May 9, 2024
8 checks passed
@vjeffrey vjeffrey deleted the vj/newresources branch May 9, 2024 13:51
@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants