Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Handling Cloudfront deletion #309

Open
wants to merge 6 commits into
base: rename_schema_for_clarity
Choose a base branch
from

Conversation

HTTP500
Copy link
Contributor

@HTTP500 HTTP500 commented Sep 23, 2020

No description provided.

@HTTP500 HTTP500 changed the base branch from master to cinq_next_dns_hijack_s3_removal September 23, 2020 01:01
case "DeleteDistribution":
return cf.analyzedeleteDistributionEntries()
default:
return &model.HijackableResourceChain{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a check here to see if its possible to create a cloud front distribution with an origin that points to a resource that doesnt exist. If this is possible, we need to check for this.

Need to check here to see if it is possible to create a distro that point to a resource that exists but we dont own.

Both cases need to have analysis code.

This may just need to be its own ticket.

@HTTP500 HTTP500 changed the base branch from cinq_next_dns_hijack_s3_removal to rename_schema_for_clarity September 29, 2020 21:15
@HTTP500 HTTP500 marked this pull request as ready for review September 29, 2020 21:49
@@ -554,6 +599,40 @@ func (r *queryResolver) GetElasticbeanstalkUpstreamHijack(ctx context.Context, e

func (r *queryResolver) HijackChainByDomain(ctx context.Context, id string, domains []string, typeArg model.Type) (*model.HijackableResourceChain, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should have its params changed.

PrimaryResource: rawChain.Resource.ID,
PrimaryAccountId: rawChain.Resource.Account,
PrimaryResourceType: rawChain.Resource.Type.String(),
PrimaryResource: root.RootResource.Account,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be resource ID?

Copy link
Contributor

@riot-jetaylor riot-jetaylor left a comment

Choose a reason for hiding this comment

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

Looks good @HTTP500

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.

2 participants