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

Can't have "template" folder in function folder #745

Open
alexellis opened this issue Dec 25, 2019 · 5 comments
Open

Can't have "template" folder in function folder #745

alexellis opened this issue Dec 25, 2019 · 5 comments

Comments

@alexellis
Copy link
Member

Can't have "template" folder in function folder

Expected Behaviour

This function stopped working at this commit, renaming template to templates fixed the issue

alexellis/cloudlogo@9e29ed5

Possible Solution

Revisit any code that does a blanket ban on copying "template" or "build" folders in the pre-build stage.

not-present

@LucasRoesler
Copy link
Member

LucasRoesler commented Dec 25, 2019

This was done in #524 to fix #478

The relevant line is here

case "build", "template":

We could probably change the check to inspect the abs path and if it is equal to abs(./templates) or abs(./build), then we skip it. But this is odd because the current code isn't actually recusing through the directory and selecting stuff, it only reads the cwd and should only skip templates and build at the top level. The CopyFiles method should be agnostic and simply copy files that are found.

Also the build output should indicate that the folder is being skipped

fmt.Printf("Skipping \"%s\" folder\n", info.Name())

@alexellis
Copy link
Member Author

I remember, thank you for the links to the issues/PR. I think this will lead to bugs that are nasty to debug if left as is.

Would you like to write some tests to prove the issue and then fix?

@rgee0
Copy link
Contributor

rgee0 commented Mar 14, 2021

I'm not sure this is as simple as it seems on first glance owing to the decision to support the use case in #478.

Consider a scenario where the handler resides in the same folder as the stack, for the (broken) cloudlogo project this would mean:

cloudlogo
├── README.md
└── clouddemo
    ├── cloud.png
    ├── handler.go
    ├── stack.yml
    └── template
        └── index.html

Here the user would build from within cloudlogo/clouddemo, and the first thing faas-cli would do is:

$ faas build
[0] > Building clouddemo.
[0] < Building clouddemo done in 0.00s.
[0] Worker done.

Total build time: 0.00s
Errors received during build:
- language template: go not supported, build a custom Dockerfile

The template folder already exists so faas-cli will not take any action without --overwrite, which isnt available on build so we have to add a second step:

$ faas template pull --overwrite
Fetch templates from repository: https://github.com/openfaas/templates.git at 
2021/03/14 12:43:47 Attempting to expand templates from https://github.com/openfaas/templates.git
2021/03/14 12:43:48 Fetched 13 template(s) : [csharp dockerfile go java11 java11-vert-x node node12 node14 php7 python python3 python3-debian ruby] from https://github.com/openfaas/templates.git

Which leaves the user with:

cloudlogo
├── README.md
└── clouddemo
    ├── build
    │   └── clouddemo
    │       ├── Dockerfile
    │       ├── function
    │       │   ├── cloud.png
    │       │   ├── handler.go
    │       │   ├── stack.yml
    │       │   └── templates
    │       │       └── index.html
    │       ├── go.mod
    │       ├── main.go
    │       └── template.yml
    ├── cloud.png
    ├── handler.go
    ├── stack.yml
    └── template
        ├── csharp
        │   ├── Dockerfile
        │   ├── Program.cs
        │   ├── function
        │   │   ├── Function.csproj
        │   │   └── FunctionHandler.cs
        │   ├── root.csproj
        │   └── template.yml
        ├── dockerfile
        │   ├── function
        │   │   └── Dockerfile
        │   └── template.yml
        ├── go
        │   ├── Dockerfile
        │   ├── function
        │   │   └── handler.go
        │   ├── go.mod
        │   ├── main.go
        │   └── template.yml
        ├── index.html
        ├── java11
        │   ├── Dockerfile
        │   ├── README.md
        │   ├── build.gradle
        │   ├── function
        │   │   ├── build.gradle
        │   │   ├── gradle
        │   │   │   └── wrapper
        │   │   │       ├── gradle-wrapper.jar
        │   │   │       └── gradle-wrapper.properties
        │   │   ├── gradlew
        │   │   ├── gradlew.bat
        │   │   ├── settings.gradle
        │   │   └── src
        │   │       ├── main
        │   │       │   └── java
        │   │       │       └── com
        │   │       │           └── openfaas
        │   │       │               └── function
        │   │       │                   └── Handler.java
        │   │       └── test
        │   │           └── java
        │   │               └── HandlerTest.java
        │   ├── gradle
        │   │   └── wrapper
        │   │       ├── gradle-wrapper.jar
        │   │       └── gradle-wrapper.properties
        │   ├── settings.gradle
        │   └── template.yml
        ├── java11-vert-x
        │   ├── Dockerfile
        │   ├── README.md
        │   ├── build.gradle
        │   ├── entrypoint
        │   │   ├── build.gradle
        │   │   ├── settings.gradle
        │   │   └── src
        │   │       ├── main
        │   │       │   └── java
        │   │       │       └── com
        │   │       │           └── openfaas
        │   │       │               └── entrypoint
        │   │       │                   └── App.java
        │   │       └── test
        │   │           └── java
        │   │               └── AppTest.java
        │   ├── function
        │   │   ├── build.gradle
        │   │   ├── settings.gradle
        │   │   └── src
        │   │       ├── main
        │   │       │   ├── java
        │   │       │   │   └── com
        │   │       │   │       └── openfaas
        │   │       │   │           └── function
        │   │       │   │               └── Handler.java
        │   │       │   └── resources
        │   │       │       └── webroot
        │   │       │           └── index.html
        │   │       └── test
        │   │           └── java
        │   │               └── HandlerTest.java
        │   ├── settings.gradle
        │   └── template.yml
        ├── node
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── handler.js
        │   │   └── package.json
        │   ├── index.js
        │   ├── package.json
        │   └── template.yml
        ├── node12
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── handler.js
        │   │   └── package.json
        │   ├── index.js
        │   ├── package.json
        │   └── template.yml
        ├── node14
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── handler.js
        │   │   └── package.json
        │   ├── index.js
        │   ├── package.json
        │   └── template.yml
        ├── php7
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── composer.json
        │   │   ├── php-extension.sh
        │   │   └── src
        │   │       └── Handler.php
        │   ├── index.php
        │   ├── php-extension.sh-example
        │   ├── readme.md
        │   └── template.yml
        ├── python
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── handler.py
        │   │   └── requirements.txt
        │   ├── index.py
        │   ├── requirements.txt
        │   └── template.yml
        ├── python3
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── __init__.py
        │   │   ├── handler.py
        │   │   └── requirements.txt
        │   ├── index.py
        │   ├── requirements.txt
        │   └── template.yml
        ├── python3-debian
        │   ├── Dockerfile
        │   ├── function
        │   │   ├── __init__.py
        │   │   ├── handler.py
        │   │   └── requirements.txt
        │   ├── index.py
        │   ├── requirements.txt
        │   └── template.yml
        └── ruby
            ├── Dockerfile
            ├── Gemfile
            ├── function
            │   ├── Gemfile
            │   └── handler.rb
            ├── index.rb
            └── template.yml

64 directories, 107 files

We can see the user's template is now in with the OpenFaaS templates. Now if the user tries to build the CLI will not copy the template folder. If changes were made to copy the template folder then the OpenFaaS templates would be copied into the resulting function image. Furthermore, there is a proposal in #849 to clobber the template folder when overwriting, which if adopted would potentially regress any solution here.

I think much of the conversation on #478 remains relevant here, particularly @LucasRoesler's query as to whether to support the handler and stack being co-located. Failing that, the idea of using .folders to separate the OpenFaaS and userland contexts seems like a cleaner alternative to iterating through a template folder to fish out anything that differs from the OpenFaaS templates.

@LucasRoesler
Copy link
Member

I have a rather big proposal, but one i have thought about for awhile and can implicitly fix this problem as well.

what if we mimicked some of the behavior we see with go modules? Specifically:

  1. a templates.yaml, this has a format <template name>: <github repo>#<sha>
  2. a templates.lock or templates.sum which has the format <github repo>#<sha> <hash>
  3. write the templates cache to a location dictated by OF_TEMPLATES_CACHE, defaulting to .openfaas/templates or something similar
  4. the template cache writes template contents to folders with the structure OF_TEMPLATES_CACHE/github.com/repo/template_name@hash
  5. (this is where we start overlapping with this issue) if the current folder has a template folder we can treat it like a "vendor" folder if it contains a special folder: templates.txt.
    if this file exists, we treat the subfolders as possible templates and we ignore this folder.
    if this file does not exist, we treat it as function content

this proposal does a couple of things

First, the files templates.yaml and templates.lock are easy to generate during faas-cli pull|build, just like Go does.

Second, this new cache scheme still allows for multiple versions of a template to be used between different projects.

Third, this also reduces the number of times a template needs to be copied to a machine.

Finally, we elimate any need to guess about colocated template folders, either they have an explicity templates.txt or not, which provides all of the needed context to decide if we should copy it into the build context, the user can easily control this.

It is important to note that this change is not backwards compatible, but we can achieve compatibility by incrementing the version in the stack.yaml . We would have version: 1.0 which stays exactly as it is today and version: 2.0 which would incidate to the CLI the new template behavior.

Version 2 would also remove the configuration.templates section because this capability is implemented by the new templates.yaml|lock

This also means we can remove the gitignore for the templates folder (if it exists) is user created.

@alexellis
Copy link
Member Author

@LucasRoesler could you spin out your comment on the lock files into its own issue?

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

No branches or pull requests

3 participants