puppetlabs / puppet-dev-tools

Puppet development tools in a Docker image
Apache License 2.0
12 stars 25 forks source link

rake lint processes from the root file system #110

Closed bschonec closed 2 years ago

bschonec commented 2 years ago

Describe the Bug

When running "docker run --rm -it -v $(pwd):/repo puppet/puppet-dev-tools:latest rake lint", the entire filesystem of the container is scanned rather than (what I assume should be) just the volume that is attached "/repo".

Expected Behavior

I expected that only /repo on the container would be 'rake lint'ed.

Steps to Reproduce

Steps to reproduce the behavior:

[root@example.com a]# docker run --rm -it -v $(pwd):/repo puppet/puppet-dev-tools:latest  rake lint
Unable to find image 'puppet/puppet-dev-tools:latest' locally
latest: Pulling from puppet/puppet-dev-tools
8ba884070f61: Pull complete 
4d04114b4f3b: Pull complete 
93bfe0625d7d: Pull complete 
9ec590102c98: Pull complete 
5a34f5d6318a: Pull complete 
Digest: sha256:ac843616bf600b648e873543c6a6609dcc109559ed33ca5f858beac69ea8c52f
Status: Downloaded newer image for puppet/puppet-dev-tools:latest
(in /)
opt/puppetlabs/pdk/private/puppet/ruby/2.4.0/gems/puppet-5.0.1/lib/puppet/pops/model/ast.pp - ERROR: two-space soft tabs not used on line 29
opt/puppetlabs/pdk/private/puppet/ruby/2.4.0/gems/puppet-5.1.0/lib/puppet/pops/model/ast.pp - ERROR: two-space soft tabs not used on line 29
opt/puppetlabs/pdk/private/puppet/ruby/2.4.0/gems/puppet-5.2.0/lib/puppet/pops/model/ast.pp - ERROR: two-space soft tabs not used on line 29
opt/puppetlabs/pdk/private/puppet/ruby/2.4.0/gems/puppet-5.3.7/lib/puppet/pops/model/ast.pp - ERROR: two-space soft tabs not used on line 29
^Crake aborted!

Environment

Red Hat Enterprise Linux release 8.5 (Ootpa)

Additional Context

I've tried with multiple version of the Docker image but they all behave the same way. I've tried running the image in interactive mode and running 'rake lint' manually but rake doesn't seem to have a way to specify the path to where you want it to lint-ify.

bschonec commented 2 years ago

As an update, I've compiled the docker image from scratch and the behavior is the same: rake starts from the root of the image's filesystem instead of /repo.

TJM commented 2 years ago

The Dockerfile has significant sadness in it. The last section that tries to remove a bunch of files becomes the final image (I think), which is also silly because if they files were not removed in the same layer they were added, they are already part of the image. Docker images are like ogres, layers! ;)

REF: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#use-multi-stage-builds

Additionally, I think it is kinda funny that lint is finding problems with Puppet Inc's own code ;)

~tommy

bschonec commented 2 years ago

What I don't understand is why the default behavior for the rake lint is to hit the root filesystem of the container. I've tried older versions and the behavior is the same. It's almost like nobody is really using this project for linting because the problem has existed for a while but the project is still getting updates.

bschonec commented 2 years ago

The README.md instructions for running the docker container do not match the testing in https://github.com/puppetlabs/puppet-dev-tools/blob/main/tests/run_tests.sh.

I'm betting that the documentation needs updating and this wasn't discovered because the (rake?) tests work properly.

TJM commented 2 years ago

@bschonec - The default behavior when you run a command like "rake lint" is to use the "current working directory" that the process starts from, which is by default in a docker container is / (root). There is a parameter in Dockerfile (WorkDir) to set the default working directory to something more useful, like /repo, but that is being set on the previous container (each "FROM" starts a new container, only the last one is published). This Dockerfile is in need of some docker expertise. The documentation definitely is the intended way for the tool to work, so that means that whoever modified the tests to not match the documentation cheated. ;

~tommy

TJM commented 2 years ago

Actually strike everything I just said, they are using "FROM base" (which is a continuation)... bah. not paying attention.

bschonec commented 2 years ago

111

eputnam commented 2 years ago

thanks for raising this, this behavior is a little unexpected. if either of you come up with anything, i think we'd be open to making a change, but unfortunately we don't have a lot of bandwidth to spend on it. specifying the Rakefile with -f seems to be the workaround for now. also, thanks @TJM for pointing out the file removal in the Dockerfile.

TJM commented 2 years ago

Hi @eputnam ... I do not have a solution either, we run ours as part of a Jenkins pipeline...

        stage('tests') {
            failFast false
            parallel {
                // Skipping PDK for now, the container doesn't have all the gems, so it takes longer
                // stage('pdk validate') {
                //     steps {
                //         //sh 'pdk validate puppet'
                //     }
                // }
                stage('puppet-code') {
                    steps {
                        sh 'puppet-lint site manifests'
                        sh 'puppet parser validate manifests site/role site/profile'
                        sh 'puppet parser validate --tasks site/patchy'
                    }
                }
                stage('Puppetfile') {
                    steps {
                        sh 'r10k puppetfile check Puppetfile'
                        sh 'r10k puppetfile check style Puppetfile'
                    }
                }
                stage('yaml-lint') {
                    steps {
                        sh 'gem install yaml-lint'
                        sh 'yaml-lint hieradata'
                    }
                }

... so, now that I look, we aren't using the rake tasks. (sigh)

eputnam commented 2 years ago

yeah, I was going to mention that pdk validate will get you linting as well and doesn't require the Rakefile.