Fix pinning of machine_type

[dimisjim]

what

Achieve pinning of COS image by fixing behavior of machine_image var and input to downstream container module

why

• Be able to pin image and not always use latest one

references

• Closes Instance being replaced even when the machine_image is pinned #114 (which needs to be reopened to be closed again with this)

[dimisjim]

remote: Permission to dimisjim/terraform-gce-atlantis.git denied to github-actions[bot].
fatal: unable to access 'https://github.com/dimisjim/terraform-gce-atlantis/': The requested URL returned error: 403

not sure what it causing exactly the error in the CI run but I have just enabled workflows in the forked repo, so maybe you can try running it again 👍

[d-costa]

@dimisjim that error is due to the terraform-docs trying to fixing the docs, and fails to commit - which makes sense
we should probably disable the git-push flag when the workflow runs on forks

#150 should fix this issue

[d-costa]

@dimisjim, this docker oneliner should inject the fix locally:

docker run --rm --volume "$(pwd):/terraform-docs" -u $(id -u) quay.io/terraform-docs/terraform-docs:0.17.0 markdown --output-file README.md --output-mode inject /terraform-docs

Could you please run this, rebase and squash the commits?

Source: https://github.com/terraform-docs/terraform-docs?tab=readme-ov-file#using-docker

[dimisjim]

cool, should be done 👍

[bschaatsbergen]

Hey @dimisjim, thanks for taking the time to submit this pull request - good catch! 👏🏼

What would occur if a user doesn't specify the machine_image variable? Given that the default is null, I anticipate the following error: Invalid value for "str" parameter: argument must not be null. Please correct me if I'm mistaken.

[dimisjim]

Thanks for the feedback @bschaatsbergen

Good catch!

Fix is provided here: dimisjim@ea2a22a

[d-costa]

LGTM

[bschaatsbergen]

Sorry for the delayed review on my side, I need to test whether this could be a breaking change or not.. have you already tested it on your side @dimisjim?

[dimisjim]

@bschaatsbergen yes, works as expected

[dimisjim]

Hello,

Is something blocking this?

[joe1981al]

Might I suggest also parsing cos_project from the input? It would be similar to what you've done but would be second item in the split, otherwise "cos-cloud" which is the default in the module being used. Not sure if necessary, but might be useful.

[d-costa]

@bschaatsbergen can you take a look at this when possible? Would be great to merge this and release a new version

[d-costa]

Hey @dimisjim sorry for the delay. Could you please squash these commits? There are some that are not signed off as required by the DCO check.

[d-costa]

I'd also suggest making data "google_compute_image" "cos" conditional based on the presence of var.machine_image, to avoid reading it even when machine_type is specified

[cblkwell]

Hey folks, we really need to get this new version out -- can we consider replicating this change in another PR if @dimisjim isn't able to fix the commit signing issue?

[dimisjim]

@cblkwell opened a new one: #172