NotificationsYou must be signed in to change notification settings
Fork906
Star10k

fix: persist terraform modules during template import#17665

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

aslilac merged 2 commits intomainfromlilac/persist-terraform-modules

May 8, 2025

Merged

fix: persist terraform modules during template import#17665

aslilac merged 2 commits intomainfromlilac/persist-terraform-modules

May 8, 2025

Conversation

Copy link

Member

aslilac commentedMay 2, 2025•
edited
Loading

We need to save the source code of the terraform modules at import, so that they're available for evaluation with dynamic parameters.

The provisioner downloads the modules during the plan, which means we can have it create a tar file, send it to the backend, and then we can persist it in the database for later use.

github-actionsbot assignedaslilac

May 2, 2025

aslilac requested a review fromEmyrk

May 5, 2025 22:43

aslilac marked this pull request as ready for review

May 5, 2025 22:45

aslilac requested review fromspikecurtis andjohnstcn ascode owners

May 5, 2025 22:45

spikecurtis reviewed

May 6, 2025

View reviewed changes

provisionerd/proto/provisionerd.protoShow resolvedHide resolved

Emyrk reviewed

May 6, 2025

View reviewed changes

coderd/provisionerdserver/provisionerdserver.goShow resolvedHide resolved

Emyrk reviewed

May 6, 2025

View reviewed changes

coderd/provisionerdserver/provisionerdserver.go OutdatedShow resolvedHide resolved

provisioner/terraform/modules.go OutdatedShow resolvedHide resolved

Emyrk reviewed

May 6, 2025

View reviewed changes

Copy link

Member

Emyrk left a comment•
edited
Loading

Choose a reason for hiding this comment

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

~~removed~~

Emyrk reviewed

May 6, 2025

View reviewed changes

Copy link

Member

Emyrk left a comment•
edited
Loading

Choose a reason for hiding this comment

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

@spikecurtis is adding a new field to the proto struct backwards compatible like injson? Meaning, does the new field default tonil?

I will ✔️ once I know it's backwards compatible 👍

provisioner/terraform/modules_internal_test.go OutdatedShow resolvedHide resolved

coderd/provisionerdserver/provisionerdserver.go OutdatedShow resolvedHide resolved

Emyrk reviewed

May 6, 2025

View reviewed changes

provisionerd/proto/version.goShow resolvedHide resolved

Copy link

Contributor

spikecurtis commentedMay 7, 2025

@spikecurtis is adding a new field to the proto struct backwards compatible like injson? Meaning, does the new field default tonil?

Yes, it's back compatible at the proto layer, so only a minor version bump, assuming the server side will tolerate it being unset. Can't recall whetherbytes is a zero-length slice or literallynil, if the other side doesn't support it, butlen(job.ModuleFiles) will be 0 in either case.