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

compute: create default firewall rules #915

Conversation

stephenplusplus
Copy link
Contributor

Related http://stackoverflow.com/questions/31509722/how-do-i-enable-http-traffic-for-gce-instance-templates
Fixes stephenplusplus/gcloud-deploy#4

We had a "maybe-bug" in our code that could lead to a user thinking they made a VM that accepts HTTP and/or HTTPS connections, but in actuality doesn't.

My theory about how the Dev Console UI works:

If you create an Instance and select "Allow HTTP traffic" and/or "Allow HTTPS traffic", it will create firewall rules in the default network: default-allow-http and default-allow-https. The VM is created with tags to match http-server and https-server, which are the target tags on the firewall rules.

When I developed the behavior of zone.createVM() that accepts config.http = true and config.https = true, I assumed all project's networks had these firewall rules by default. But, if my theory is right, I only had them because I used the UI to create an http/https VM before, so my project had the rules.

So, this PR will attempt to create default-allow-http and default-allow-https (when that functionality is asked for), and if they already exist, it just moves on and creates the VM. If they don't exist, it creates them, then the VM. In other words, this should basically work like the Dev Console UI does (if my theory is correct).

@stephenplusplus stephenplusplus added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. api: compute Issues related to the Compute Engine API. labels Oct 17, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 17, 2015
@jgeewax
Copy link
Contributor

jgeewax commented Oct 17, 2015

LGTM

@stephenplusplus
Copy link
Contributor Author

@callmehiphop PTAL

callmehiphop added a commit that referenced this pull request Oct 19, 2015
…-firewall

compute: create default firewall rules
@callmehiphop callmehiphop merged commit 6ceb711 into googleapis:master Oct 19, 2015
@callmehiphop
Copy link
Contributor

Looks good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: compute Issues related to the Compute Engine API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants