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

allow vpc peering to accept explicit VPC IDs instead of friendly names #428

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

keen99
Copy link
Contributor

@keen99 keen99 commented Jan 8, 2016

to avoid lookups against resources we might not maintain - #427

to avoid lookups against resources we might not maintain - chef-boneyard#427
@joaogbcravo
Copy link
Contributor

Hey!
#416 fix this problem!

Cheers,
João

@keen99
Copy link
Contributor Author

keen99 commented Jan 13, 2016

I saw yours after I'd already built mine (probably because I only searched issues, not PRs...) - but you didn't cover the update case, only create. ie update_aws_object - https://github.com/chef/chef-provisioning-aws/blob/13e69c444b95aac2f41ee7f0bd307f13bba33734/lib/chef/provider/aws_vpc_peering_connection.rb#L58-L59

See issue #427 for more detail.

@keen99
Copy link
Contributor Author

keen99 commented Jan 13, 2016

also, #428 goes a bit further by allowing both "vpc" and "peer_vpc" to be specific IDs and avoid lookups (since you might want to define a peering relationship on a VPC not created/maintained by chef)

@joaogbcravo
Copy link
Contributor

Hey,

Probably I'm not correctly seeing problem and the resolution, but #416 have the following code:

    if new_resource.peer_owner_id.nil?
      options[:peer_vpc_id] = new_resource.peer_vpc
      options = AWSResource.lookup_options(options, resource: new_resource)
    else
      options = AWSResource.lookup_options(options, resource: new_resource)
      options[:peer_vpc_id] = new_resource.peer_vpc
      options[:peer_owner_id] = new_resource.peer_owner_id
    end

So it will only lookup if you don't set the peer_owner_id (meaning you own the other VPC). Even if it lookup, you can set the specific ID. Its for that reason that lookup works, right? You can put the id or an object and it will identify what you want.

@tyler-ball
Copy link
Contributor

Hey y'all! I wrote a test to demonstrate this error, but cannot recreate it. Here is my test:

with_aws "with 2 VPCs not owned by Chef Provisioning" do
  vpc1 = nil
  vpc2 = nil
  before do
    converge do
      vpc1 = driver.ec2_resource.create_vpc(cidr_block: '10.0.0.0/24')
      vpc2 = driver.ec2_resource.create_vpc(cidr_block: '11.0.0.0/24')
      vpc1.wait_until {|v| v.state == 'available'}
      vpc2.wait_until {|v| v.state == 'available'}
    end
  end

  after do
    converge do
      vpc1.delete
      vpc2.delete
    end
  end

  it "aws_peering_connection 'test_peering_connection' with minimal parameters creates a active connection" do
    expect_recipe {
      aws_vpc_peering_connection 'test_peering_connection' do
        vpc vpc1.id
        peer_vpc vpc2.id
      end
    }.to create_an_aws_vpc_peering_connection('test_peering_connection',
      :'requester_vpc_info.vpc_id' => vpc1.id,
      :'accepter_vpc_info.vpc_id' => vpc2.id,
      :'status.code' => 'active'
    ).and be_idempotent
  end
end

What am I doing wrong that I cannot recreate this error?

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

Successfully merging this pull request may close these issues.

3 participants