Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with duct/include reader macro / config.edn #23

Open
rune-brinckmeyer opened this issue Jun 7, 2019 · 5 comments
Open

Issue with duct/include reader macro / config.edn #23

rune-brinckmeyer opened this issue Jun 7, 2019 · 5 comments

Comments

@rune-brinckmeyer
Copy link

Just started fiddling with duct and integrant, so please bear with a beginner here. I have stumbled upon what I believe may be an issue related to the duct/include reader macro.

I created a standard duct project. It works as expected from the repl:

(dev)
=> :loaded
(prep)
=> :prepped

When I embed the same files in a docker container (clojure:openjdk-8) under
/home/runner/backend and run the repl yields this unexpected result:

(dev)
=> :loaded
(prep)
Execution error (FileNotFoundException) at java.io.FileInputStream/open0 (FileInputStream.java:-2).
/home/runner/backend/dev (Is a directory)

This error eventually leads to this line in resources/backend/config.edn:

:duct.profile/dev #duct/include "dev"

Following the implementation of the duct/include reader macro reveals that duct/include processes argument "dev" through config-resource:

(defn- config-resource [path]
 (or (io/resource path)
   (io/resource (str path ".edn"))
   (io/resource (str path ".clj"))))

Investigating this from the repl in the working project I find that:

(io/resource "dev")
=> nil
(io/resource (str "dev" ".edn"))
=> #object[java.net.URL 0x54458573 "file:/home/runeb/backend/dev/resources/dev.edn"]

while inside the container the repl gives me:

(io/resource "dev")
=> #object[java.net.URL 0x7580ef78 "file:/home/runner/backend/dev"]
(io/resource (str "dev" ".edn"))
=> #object[java.net.URL 0x29040c80 "file:/home/runner/backend/dev/resources/dev.edn"]

clearly when in the docker container (io/resource path) maps to the dev directory in the project root which is not a slurp-able resource (slurped in duct/make-include):

(defn- make-include [readers]
  (fn [path]
    (let [opts {:readers (merge-default-readers readers)}]
      (some->> path config-resource slurp (ig/read-string opts)))))

One must conclude that the project root is somehow added to the resource search list inside the container. I sure did not put it there, and my Java-fu is not good enough to perceive how it got there, or if it is supposed to be there in the first place.

Fixing this is easy enough by being more precise with with the include statement in config.edn like this:

:duct.profile/dev     #duct/include "dev.edn"      ;; dev.edn more specific than dev

The questions that arise are:

  1. Is this fix the rigtht way to do it? Is there a reason for duct/include to refer to "dev" instead of "dev.edn" in the config.edn? (Since this is what is resolves to anyway?)
  2. Any clue if there is something inherently wrong with resource path?
    my project.clj mentions only
    :resource-paths ["resources" "target/resources"]
    :resource-paths ["dev/resources"]
    so I have no clue how the project root (containing the project.clj file) got into the mix..

Any enlightenment is highly appreciated :-)

@weavejester
Copy link
Contributor

Thanks for finding and investigating this issue.

Are you running this via lein repl? I'm surprised that the docker container would add the current directory to what I assume is the system classpath. That sounds like it would have all manner of security concerns.

Could you provide the command you used to run the docker container?

@rune-brinckmeyer
Copy link
Author

@weavejester Yes, its run through standard lein repl.

I have set it up through docker-compose, but the corresponding docker command would be

sudo docker run -it backend lein repl

@weavejester
Copy link
Contributor

What image are you using? clojure:openjdk-8-lein?

@rune-brinckmeyer
Copy link
Author

Yes. clojure:openjdk-8-lein-2.9.1 to be precise :-)

@kwrooijen
Copy link
Contributor

Hi, I'd like to contribute to this discussion. I was planning on creating a new issue, but stumbled upon this one. I already wrote my issue offline so I'll post it here:

The duct/include reader accepts a string (for example "test"). The
reader will then look for a file in resources named "test", "test.edn", and
"test.clj", in that order. Currently the convention is to simply type the name
of the file without extension (based on the duct-template). For example:

:duct.profile/dev   #duct/include "dev"
:duct.profile/local #duct/include "local"

When trying to add a test profile, I add it the same way.
Test profile introduced in 0.8.0

:duct.profile/test #duct/include "test"

But instead of including the test.edn that I created, I include a muuntaja resource from my dependencies:

(clojure.java.io/resource "test")
#object[java.net.URL 0x14688c7 "jar:file:/Users/kevin/.m2/repository/metosin/muuntaja/0.6.3/muuntaja-0.6.3.jar!/test"]

This is really problematic for Ducts design / conventions. This will cause users
to trip over this issue, or if you update a dependency you might break your
system.

Does duct/include need to be able to include non edn content?

  1. We could make duct/include only include files with an edn extension.
  2. Another option is to include only absolute file names. This would mean we will have to change the template to:
:duct.profile/dev   #duct/include "dev.edn"
:duct.profile/local #duct/include "local.edn"

The second option would be a breaking change for core. However it does have my preference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants