mitchellh / vagrant-aws

Use Vagrant to manage your EC2 and VPC instances.
MIT License
2.61k stars 574 forks source link

aws.ami is forced to be defined when it shouldnt need to be #538

Open ju2wheels opened 6 years ago

ju2wheels commented 6 years ago

I have a repository manager manager for hosting Vagrant boxes (we can assume its just an HTTP server here) and I have my Vagrant boxes and a box.json on this server (not using Atlas).

The idea being that I should just be able to define vm.box and vm.box_url (pointing to box.json) and not need to define anything else for the provider (since the box has AWS provider defaults). Vagrant should then be able to get the box and create an instance.

Example Vagrantfile:

Vagrant.configure("2") do |config|
  config.vm.define "vagrant-test" do |vagrant_test|
    vagrant_test.vm.box     = 'mycompany/ubuntu-16.04-ansible-2.5.0'
    vagrant_test.vm.box_check_update = true
    vagrant_test.vm.box_url = "https://mycompany.com/box.json"

    vagrant_test.vm.provider :aws do |aws, override|
      aws.tags = {'Name' => 'vagrant-test', 'vagrant' => 'vagrant'}
    end
  end
end

Error:

There are errors in the configuration of this machine. Please fix
the following errors and try again:

AWS Provider:
* An AMI must be configured via "ami" (region: #{region})

This is unexpected because the Vagrant box retrieved by box.json will have aws.ami defined in it as a default for the provider

The above will work if I explicitly add aws.ami to the provider above to override the box provider defaults or if I already have the box installed locally then we dont have to override the box value at all.

This forced verification here seems wrong.

naviat commented 5 years ago

No one support this provider?

deft01 commented 3 years ago

Hi @ju2wheels , did you found a way to do that ?

deft01 commented 3 years ago

I would be interested in the solution.

ju2wheels commented 3 years ago

@deft01 No, unfortunately I had no choice but to add the aws.ami into the project's Vagrantfile (even though it shouldnt be needed) in addition to it being defined in the Vagrantfile in the custom AWS Vagrant box.

vohi commented 2 years ago

Running into this problem, I came up with a hack: in your box's Vagrantfile, add the box's name to a global array in the global scope, i.e. before the Vagrant.configure('2') do |config|:

$SEEN_BOXES << "mycompany/ubuntu-16.04-ansible-2.5.0"

Then in your aws-specific provider configuration in the project Vagrantfile, check if the box you want to use is in that array, and if not, set the ami to a dummy value:

aws.ami = "dummy" unless $SEEN_BOXES.include?(boxname)

What will happen is that Vagrant runs your project aws provider, will set "dummy"; it will then download the missing box file from your server, load that box's Vagrantfile, and the box is added to $SEEN_BOXES. The box's Vagrantfile's aws provider code will overwrite "dummy" with the value you want, and your project Vagrantfile's aws provider code will not overwrite that anymore.

The limitation this has is that you need to know under what name the box will be added to in the box's Vagrantfile, but for distributing your boxes internally that should be ok.

ju2wheels commented 2 years ago

I dont get how that solves the problem. The Vagrant box name and the AMI ID are two completely separate things. All you end up doing there is not setting a default AMI in the base box (by setting the default to dummy you are just creating a base box that you always have to override), which defeats making a base box for AWS in the first place since you want the base box to be tied to a specific AMI ID.

This issue is entirely tied to Hashicorp's weird obsession with nested scopes or whatever it is and how it does validation on the defaults associated with that, I forget its been a while since I did Vagrant plugin development.

vohi commented 2 years ago

@ju2wheels in the base box's Vagrantfile you set the ami, and you add the box name to the global array.

In the environment Vagrantfile, you check the global array, and if the box file has been loaded, then its name will be in the array. Then you don't overwrite the ami with "dummy". If the entry is not in the array, then set the ami to "dummy" and the plugin will be satisfied with the validation conditions, download the box file, load that box file's Vagrantfile and overwrite your "dummy" setting.

See the commits referenced above for how I do it in my project. It ain't pretty and I'd rather not need it (I don't need it in the Azure box logic), but it works.