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

add the ability to set the network property 'source/destination check' #38

Merged
merged 2 commits into from
Sep 2, 2016
Merged

add the ability to set the network property 'source/destination check' #38

merged 2 commits into from
Sep 2, 2016

Conversation

fearoffish
Copy link
Contributor

@fearoffish fearoffish commented Sep 1, 2016

The ability to set the 'source/destination check' network property is useful for instances performing cross-VPC routing. This allows a resource pool property to set this, it defaults to 'true' which is the AWS default.

- name: my-vpn-server
  cloud_properties:
    source_dest_check: false

@cfdreddbot
Copy link

Hey fearoffish!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@@ -34,6 +34,7 @@ def create(agent_id, stemcell_id, resource_pool, networks_spec, disk_locality, e
instance.wait_for_running
instance.attach_to_load_balancers(resource_pool['elbs'] || [])
instance.update_routing_tables(resource_pool['advertised_routes'] || [])
instance.source_dest_check = resource_pool['source_dest_check'] || true
Copy link
Contributor

Choose a reason for hiding this comment

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

@fearoffish afraid you can't use the fallback operator here as false || true always evaluate to true. Your unit tests back this up as you set 'source_dest_check' => false at the top, yet every test expects allow(aws_instance).to receive(:source_dest_check=).with(true). Could you fix this and add tests for both the false and true cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can do resource_pool.fetch('source_dest_check', true)

@ljfranklin
Copy link
Contributor

@fearoffish neat idea, would also allow people to deploy NAT boxes with BOSH. Added a code comment inline.

@fearoffish
Copy link
Contributor Author

Fixed. Extra points for the beginner ruby error, right? :)

@cunnie cunnie merged commit 10fd76b into cloudfoundry:master Sep 2, 2016
@cppforlife
Copy link
Contributor

Added an integration test: 3c7cb07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants