-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 firewall support safer-cluster modules #570
Add firewall support safer-cluster modules #570
Conversation
Signed-off-by: Dev <Dev25@users.noreply.github.com>
Signed-off-by: Dev <Dev25@users.noreply.github.com>
Signed-off-by: Dev <Dev25@users.noreply.github.com>
{% if not private_cluster %} | ||
depends_on = [ | ||
google_container_cluster.primary, | ||
] | ||
{% endif %} |
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.
Private clusters have a pre defined master cidr so no need to wait for cluster creation to determine master ip range.
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.
LGTM
If you have bandwidth a test would be great, I did a quick scan but didnt see any existing FW tests for any of the main modules.
Signed-off-by: Dev <Dev25@users.noreply.github.com>
@bharathkkb Never added a new inspec test before, I've added a firewall existence check based on another tf module test |
@Dev25 I think we need a new verifier system block in - name: "safer_cluster"
driver:
root_module_directory: test/fixtures/safer_cluster
verifier:
systems:
- name: safer_cluster
backend: local
- name: inspec-gcp
backend: gcp
controls:
- network |
Signed-off-by: Dev <Dev25@users.noreply.github.com>
Right thanks! Hopefully that does the trick. |
@Dev25 I just took a look at another test we have in this module and it looks like we need another small tweak, to explicitly say only the gcloud control should use local backend. PFA new version. - name: "safer_cluster"
driver:
root_module_directory: test/fixtures/safer_cluster
verifier:
systems:
- name: safer_cluster
backend: local
controls:
- gcloud
- name: inspec-gcp
backend: gcp
controls:
- network |
Signed-off-by: Dev <Dev25@users.noreply.github.com>
control "network" do | ||
title "gcp network configuration" | ||
describe google_compute_firewalls(project: project_id) do | ||
its('firewall_names') { should include "gke-#{cluster_name[0..25]}-intra-cluster-egress" } |
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.
Not a ruby expert but looking at this [1] wont we need to discard the last char as we need length to be at most 25chars? So [0...25]
or [0,25]
[1]: https://stackoverflow.com/questions/9690801/difference-between-double-dot-and-triple-dot-in-range-generation
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.
@Dev25 Saw this error in CI
Cluster name: safer-cluster-cluster-7yid
Actual FW name when I checked: gke-safer-cluster-cluster-7yi-intra-cluster-egress
What we checked for in the test: gke-safer-cluster-cluster-7yid-intra-cluster-egress
× network: gcp network configuration (2 failed)
× google_compute_firewalls firewall_names should include "gke-safer-cluster-cluster-7yid-intra-cluster-egress"
expected ["default-allow-icmp", "default-allow-internal", "default-allow-rdp", "default-allow-ssh", "gke-deplo...node-http-hc", "k8s-fw-a2ee97657700f434d9340944b00a5b61", "k8s-fw-ae0f40471c88f42199bb5196a6394f69"] to include "gke-safer-cluster-cluster-7yid-intra-cluster-egress"
× google_compute_firewalls firewall_names should include "gke-safer-cluster-cluster-7yid-webhooks"
expected ["default-allow-icmp", "default-allow-internal", "default-allow-rdp", "default-allow-ssh", "gke-deplo...node-http-hc", "k8s-fw-a2ee97657700f434d9340944b00a5b61", "k8s-fw-ae0f40471c88f42199bb5196a6394f69"] to include "gke-safer-cluster-cluster-7yid-webhooks"
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.
Changed to [0,25]
, thanks for helping debug this.
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.
LGTM
fix for lint test timing out is in #538
Closes #569