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

test: Add cloud firewall for integration tests #1444

Conversation

ykim-akamai
Copy link
Contributor

@ykim-akamai ykim-akamai commented May 28, 2024

📝 Description

This PR implements Linode Cloud Firewall for integration tests to enhance security.

Default Inbound policy: DROP
Default Outbound policy: ACCEPT
Inbound rule: inbound rule with public ip on port 22

Note: GHA does not support ipv6 so only ipv4 will get added in firewall during GHA execution. However, ipv6 will get added automatically if ipv6 address and route exist

✔️ How to Test

make int-test

📷 Preview

If applicable, include a screenshot or code snippet of this change. Otherwise, please remove this section.

@ykim-akamai ykim-akamai added the testing for updates to the testing suite in the changelog. label May 28, 2024
@ykim-akamai ykim-akamai requested a review from a team as a code owner May 28, 2024 19:15
@ykim-akamai ykim-akamai requested review from jriddle-linode and lgarber-akamai and removed request for a team May 28, 2024 19:15
@ykim-akamai ykim-akamai marked this pull request as draft May 28, 2024 19:16
@ykim-akamai ykim-akamai marked this pull request as ready for review May 30, 2024 03:23
@@ -26,3 +27,9 @@ var ProtoV5ProviderFactories = map[string]func() (tfprotov5.ProviderServer, erro
return muxServer.ProviderServer(), nil
},
}

var HttpExternalProviders = map[string]resource.ExternalProvider{
Copy link
Contributor Author

@ykim-akamai ykim-akamai May 30, 2024

Choose a reason for hiding this comment

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

Now this external provider is required for any tests that use Linode Cloud Firewall

e.g. ExternalProviders: acceptance.HttpExternalProviders,

outbound_policy = "ACCEPT"
inbound_policy = "DROP"

dynamic "inbound" {
Copy link
Contributor Author

@ykim-akamai ykim-akamai May 30, 2024

Choose a reason for hiding this comment

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

This incoming block will include either IPv4, IPv6, or both in Firewall rule, depending on their validity and availability.

@lgarber-akamai
Copy link
Contributor

GHA E2E Run: https://github.com/linode/terraform-provider-linode/actions/runs/9320639883

@lgarber-akamai
Copy link
Contributor

@ykim-1 I noticed some test configs in this PR don't implement firewalls but have been changed to disable booting. Will this be enough for us to stay compliant with the new policy?

e.g. https://github.com/linode/terraform-provider-linode/pull/1444/files#diff-6d867b4120f4b4a41637f21a7059665d5ce1dc0f2082e234ea23264ebd6a6c39R13

@ykim-akamai
Copy link
Contributor Author

GHA E2E Run: https://github.com/linode/terraform-provider-linode/actions/runs/9320639883

Thank you for tracking this :)

@ykim-akamai
Copy link
Contributor Author

ykim-akamai commented Jun 4, 2024

@ykim-1 I noticed some test configs in this PR don't implement firewalls but have been changed to disable booting. Will this be enough for us to stay compliant with the new policy?

e.g. https://github.com/linode/terraform-provider-linode/pull/1444/files#diff-6d867b4120f4b4a41637f21a7059665d5ce1dc0f2082e234ea23264ebd6a6c39R13

I think it should be fine based on conversation with John earlier last wek but, I will confirm with someone from security team.

Update: I just got notified "Up to 48H there is no need to add them to a FW, as long as there is no sensitive data there", also given disk(s) is not booted

Copy link
Member

@zliang-akamai zliang-akamai 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 to me!

Copy link
Collaborator

@jriddle-linode jriddle-linode left a comment

Choose a reason for hiding this comment

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

LGTM works locally.

@lgarber-akamai
Copy link
Contributor

@ykim-1 I noticed some test configs in this PR don't implement firewalls but have been changed to disable booting. Will this be enough for us to stay compliant with the new policy?
e.g. https://github.com/linode/terraform-provider-linode/pull/1444/files#diff-6d867b4120f4b4a41637f21a7059665d5ce1dc0f2082e234ea23264ebd6a6c39R13

I think it should be fine based on conversation with John earlier last wek but, I will confirm with someone from security team.

Update: I just got notified "Up to 48H there is no need to add them to a FW, as long as there is no sensitive data there", also given disk(s) is not booted

@ykim-1 Sounds perfect, thank you!

Copy link
Contributor

@lgarber-akamai lgarber-akamai left a comment

Choose a reason for hiding this comment

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

LGTM, excellent work!

@ykim-akamai ykim-akamai merged commit df142e6 into linode:dev Jun 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing for updates to the testing suite in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants