Terraform: Cannot get terraform to build using 'make'

Created on 19 Jul 2016  ยท  6Comments  ยท  Source: hashicorp/terraform

Hi, I'm attempting to build terraform from source using make. In fact here's a handy Dockerfile to completely demonstrate the failure:

FROM ubuntu:latest

RUN apt-get update && apt-get install -y git curl wget golang build-essential
RUN bash -c 'mkdir -p /opt/go/src/github.com/{hashicorp,mitchellh}'

ENV GOPATH=/opt/go
ENV PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/opt/go/bin

RUN ls /opt/go/src/github.com/

RUN cd /opt/go/src/github.com/hashicorp \
    && git clone https://github.com/hashicorp/terraform \
    && cd terraform \
    && git checkout v0.6.16 \
    && make

This always fails with:

--- FAIL: TestRead_PathNoPermission (0.00s)
    read_test.go:82: Expected error, got none!
FAIL
FAIL    github.com/hashicorp/terraform/helper/pathorcontents    0.003s

Is this test broken? It also fails when checking out master instead of v0.6.16. Thanks

documentation

Most helpful comment

Question: is there any ongoing relevance or reason to keep this particular test as part of the Terraform suite: it tests some basic filesystem semantics that happen to break down under Docker (and the overall trend for build/CI solutions is to be backed by Docker or docker-like setups), and special-casing the test to skip under Docker means the test probably isn't adding much.

Why not drop the test? All it's doing is writing a file, taking away permissions and then trying to read it back - and it's entirely just combining calls to core library code, and not even touching TF code. That's the kind of behavioural testing you'd expect to see inside the golang io package - and not in it's consumers. In fact, the only test I can see being overly useful in that file is actually the one that tests the resolution of ~/ paths cross-platform using the imported library.

All 6 comments

The cause was building as root, as root can read files chmodded as 000, thus causing the test to fail.

Probably in the prerequisites, documentation, it should be noted that an unprivileged user has to run "make".

Question: is there any ongoing relevance or reason to keep this particular test as part of the Terraform suite: it tests some basic filesystem semantics that happen to break down under Docker (and the overall trend for build/CI solutions is to be backed by Docker or docker-like setups), and special-casing the test to skip under Docker means the test probably isn't adding much.

Why not drop the test? All it's doing is writing a file, taking away permissions and then trying to read it back - and it's entirely just combining calls to core library code, and not even touching TF code. That's the kind of behavioural testing you'd expect to see inside the golang io package - and not in it's consumers. In fact, the only test I can see being overly useful in that file is actually the one that tests the resolution of ~/ paths cross-platform using the imported library.

@phinze , any thoughts on comments from @steve-gray regarding the TestRead_PathNoPermission.

The test complicates things for those of us working with the terraform code base in a Docker container and doesn't seem to provide any critical test coverage.

Hey folks! I had some fun trying to figure out what 3-years-ago @phinze was thinking there. Here's what I've come up with:

TestRead_PathNoPermission verifies that the pathorcontents.Read helper function properly returns an error when a path exists but is not readable. It is the test that covers the implementation's extra os.Stat before diving in to read the file.

Without this, the helper doesn't have its present-but-not-readable behavior specified one way or another, and an implementation that simply returned the path and no error for an unreadable path would pass all the other tests.

In other words, I think the test is worth keeping. I'd be fine for us to t.Skip it if the user is root though.

Hope this helps explain the context here!

Makes sense, thanks for looking into this @phinze and chasing down what 3-years-ago @phinze was doing. Also appreciate the PR to harden the test.

I'm going to lock this issue because it has been closed for _30 days_ โณ. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

Was this page helpful?
0 / 5 - 0 ratings