cloudfoundry / cf-deployment-concourse-tasks

Apache License 2.0
23 stars 76 forks source link

Incorrect IaaS used when "required" param is missing for `bosh-upload-stemcell-from-cf-deployment` #15

Closed micahyoung closed 3 years ago

micahyoung commented 7 years ago

When the INFRASTRUCTURE param is omitted, this param defaults to google which seems like an unsafe default. This should raise an error instead if it is actually required.

I spent quite a bit of time debugging as this only presented itself in the subsequent job that failed.

https://github.com/cloudfoundry/cf-deployment-concourse-tasks/blob/64f432d/bosh-upload-stemcell-from-cf-deployment/task.yml#L24

cf-gitbot commented 7 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/144722901

The labels on this github issue will be updated when the story is started.

dsabeti commented 7 years ago

Hey @micahyoung. Thanks for this feedback. Overall, your request makes sense, but I suspect that many people are already relying on the default value to be google, so changing this would break them. What if the task provided a warning, but continued to operate the same way (using google as the default)? This should at least stop users who haven't configured the INFRASTRUCTURE param from wasting a bunch of time debugging. Would that help?

Otherwise, we can remove the default and bump the major version of the repo.

davewalter commented 3 years ago

Closing due to inactivity.