-
Notifications
You must be signed in to change notification settings - Fork 138
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
Determine and document expected directory structure for language pack extensions #452
Labels
Comments
lance
added
kind/enhancement
MVP
language-packs
Issues related to built in and extensible language packs
labels
Jul 30, 2021
This was referenced Jul 30, 2021
lance
added a commit
to lance/func
that referenced
this issue
Sep 20, 2021
Adds more detail around how language packs can be provided by third parties, and the expected structure and metadata required. Fixes: knative#452 Signed-off-by: Lance Ball <lball@redhat.com>
lance
added a commit
to lance/func
that referenced
this issue
Sep 30, 2021
Adds more detail around how language packs can be provided by third parties, and the expected structure and metadata required. Fixes: knative#452 Signed-off-by: Lance Ball <lball@redhat.com>
lance
added a commit
to lance/func
that referenced
this issue
Oct 20, 2021
Adds more detail around how language packs can be provided by third parties, and the expected structure and metadata required. Fixes: knative#452 Signed-off-by: Lance Ball <lball@redhat.com>
knative-prow-robot
pushed a commit
that referenced
this issue
Oct 27, 2021
…558) * feat!: add support for manifest.yaml and runtime.yaml BREAKING CHANGE This commit adds the concepts of a manifest.yaml file at the top level of a template repository, and a runtime.yaml file for each language/runtime within the repository. Prior to this, repository metadata was primarily restricted to information that could be gleaned from the file system, such as the directory name used as the Name. A manifest.yaml file contains a Name, URL, Version, and a list of supported runtimes; and if all of the runtimes expose the same health endpoints, these can be set here. Within a runtime directory, there may exist a runtime.yaml file which allows language pack providers to set the health endpoints, overriding the values set in manifest.yaml, as well as Builders and Buildpacks. This change removes support for a builders.yaml file in a runtime/template directory, as this was guaranteed to be redundant with the builders.yaml file in a sibling template directory. The choice was to move these up a level to runtime/runtime.yaml. Signed-off-by: Lance Ball <lball@redhat.com> * fixup: bump go version for unit tests Signed-off-by: Lance Ball <lball@redhat.com> * fixup: using go 1.16 now requires 'go mod tidy' See: https://golang.org/doc/go1.16 Signed-off-by: Lance Ball <lball@redhat.com> * fixup: restore support for directory traversal Signed-off-by: Lance Ball <lball@redhat.com> * fixup: whitespace linting Signed-off-by: Lance Ball <lball@redhat.com> * fixup: update func_yaml-schema.json Signed-off-by: Lance Ball <lball@redhat.com> * fixup: spelling errors caught by reviewdog Signed-off-by: Lance Ball <lball@redhat.com> * fixup: use os.ReadDir Signed-off-by: Lance Ball <lball@redhat.com> * squash: restore project creation from remote repos Signed-off-by: Lance Ball <lball@redhat.com> * fixup: remove billyFilesystem Signed-off-by: Lance Ball <lball@redhat.com> * doc: expand on language pack documentation Adds more detail around how language packs can be provided by third parties, and the expected structure and metadata required. Fixes: #452 Signed-off-by: Lance Ball <lball@redhat.com> * doc: update language pack documentation Signed-off-by: Lance Ball <lball@redhat.com> * fixup: cleanup deps Signed-off-by: Lance Ball <lball@redhat.com> * fixup: cleanup deps - again Signed-off-by: Lance Ball <lball@redhat.com> * fixup: reviewdog whitespace cleanup Signed-off-by: Lance Ball <lball@redhat.com> * fixup: custom repo template listing Signed-off-by: Lance Ball <lball@redhat.com> * fixup: bump go version for integration tests Signed-off-by: Lance Ball <lball@redhat.com> * fixup: adjust test-templates repo uri for e2e Signed-off-by: Lance Ball <lball@redhat.com> * fix: don't fail when extended templates don't support a given runtime Signed-off-by: Lance Ball <lball@redhat.com> * src: repo and template type hierarchy As the client API increases in complexity, upgrading to a more correct type hierarchy for the object managers serves to keep things clean. In this update, the Repository and Templates manager now use constructors with private members, including a backreference to the root Client object, which provides the mangers with full access to the current client instance API for implementing their (growing) features. * src: template manager writes The templateWriter struct has now been upgraded to integrate with the newly-added type system which includes a Templates Manager by adding a .Write method which writes a template at the given location. This encapsulation of the write functionality moves us towards having a cleaner abstraction atop writing any template to disk from any repository. In addition, the "Get" prefix was removed from many accessor methods, as this is standard a Go idiom. * src: templates write including denormalize - merges templateWriter into templates manager - denormalization of builders, health endpoints etc. made part of .Write - write logic now works on a Template rather than string paths etc The templates manager can now write out a template given a function directy. Included internnaly to write is the denormalization logic, which is now closer to where it is used, which should improve cohesion and decouple the template implementation detail from the Client. Also moves the write logic closer to the template itself, with the expectation the same will happen for functions when Config and Function are merged. * src: vendor * src: merge manifests into domain model In general, the manifests sytsem is merged more tightly into the overall system while also expalding it to use a path to templates to avoid having to create a full filesystem shadow-copy in yaml. Preserves backwards-compatiblity by keeping the base case a yaml-free repo of only templates. Creaets a hierarchical inheritance from repository to runtime to template for builders, endpoints etc. whcih allows for the manifest within templates to be a first-class citizen, though its use will be rather rare. Some more in-depth explanations of the more substantive changes are as follows: - Merges Manifest with Repository The Manifest structure is essentially a serialized Repository, so they are now the same object. - Converts repo URL to a calculated field The URL is informative and entirely dependent on the current state of the repo, so it should be calculated to avoid confusion and avoid being serialized to the manifest. - Merge FunctionTemplate with Template The struct being serialized here is a Template, so they are now the same object - Extracts new "BuildConfig" embedded struct The Builders and Buildpacks members are shared between Repository, Runtime and Template, so they are now an embedded stuct in the same manner as HealthEndpoints. - yaml 'inline' declaration for embedded structs The HealthEndpoints and other embedded structs were not actually being read from their configs because of a nuance of yaml which (in contrast to json unmarshallers) requires the `,inline` tag on the struct member to trigger inclusion in decode. - Replaces Repository's runtimes member with templates While it is true that on disk templates are subdivided into their effective runtime, this is for ease of development. The logical structure is: repositories have templates. Templates are applicabe to runtimes. Therefore the correct API is to have templates a direct member of repository but requiring a runtime filter for access. - Use repository templates path instead of full list The goal is to support the use of repositories which contain more than just templates. The solution is to define an alternate location for templates (such as ./templates). Creating an entire exhaustive manifest where each runtime and each template is defined with a name and path is a usurpation of the responsibilities of a filesystem, and causes unintend negative knock-on effects. Let's try this simple solution before jumping to the nuclear option of defining a filesystem-in-yaml. - Use filesystems when loading repository objects The fact that a file exists on the disk, embedded, or remote should be of no concern to most of the code. This is accomplished by using the filesystem abstraction which is used during template writing. This will be further improved in future versions by upgrading to use the fs.FS interface and tooling. - Uses manifest.yaml at all levels Once the hierarchical nature of the manifest was instituted, it seemed more intuitive to have the same filename at each level. This also has the nice benefit of being backwards-compatible with template-level manifest. This is not a correctness issue, but a guess that remembering a single filename which can exist at different levels in a hierarchy is easier to remember than different filenames, and its location in the hierearchy sufficient differentiator making the different filenames _perhaps_ unencssary. The constants are left as separate for an easy revert. - Moves static defaults into code wherever possible Wherever yaml was being statically compiled in, it was updated such that canonical values for static defaults were defined on the Go structure, with the yaml being for overrides. For example Default Readiness and Liveness endpoints. Some notable benefits of letting the filesystem do it's thing: * Preserves the ability to create a repository of templates without hacking yaml * Reflects changes directly in IDEs which have native support for filesystem, not so for a proprietary manifest.yaml * the builtin repository has no name, so parsing a manifest from within it is both prone to error and confusing, as it could lead future developers to submit PRs which change its name. * fix: revert repository.git to a bare server repo * src: repository default name The Repository struct now includes a Name and DefaultName. The former is the current name of the repository, and corresponds to the path on disk. The latter is the name specified in the manifest.yaml, and is used as the default name of the repository when no name is provided. Fixes an issue where there could be name collisions using a uuid for the initial name until the manifest is read in. Restructures the embedded repositories such that one exemplifies a base case repo with no metadata and only templates, the other specifying a manifest to exemplify for example a complete language pack. Adds a test such that all three cases are covered: explicit name, no name but manifest-defined default name, URI-derived name. * feat: single repo mode Enables single repository mode suystem-wide, which fully implements the logic intended by the WithRepository option. The default repoisitory is now programatically defined, and while by default is the embedded repo, it can be overriden using the WithRepository option. This keeps the logic internal to the repositories managet, removeing the need for a crossp-cutting concern in other parts of the system. * src: manifest inheritance and existence tests Respects alternate templates location when defined in a manifest. Respects manifest embedded structs HealthEndpoints and BuildConfig at each of the three levels, with inheritance: Repo, Runtime and Template. Separates test repositories into the two cases of a templates-only repo (with no manifest) and a complex repo (with manifests) such as for language packs. Leaves the BuildConfig struct as inline but sets the HealthEndpoints as being named, such that the latter has its fields in the yaml under the key 'healthEndpoints' but the former does not (passthrough). Adds error checks when attempting to access named repo when in single-repo mode. * src: remove function merge Removes feature of mergine a func.yaml defined in a template in favor of sticking with the manifest.yaml until such time as this is requested. Assuming YAGNI. * src: repo filesystems - Embeds repositories with an internal filesystem - Replaces filesystem disk checkouts with in-memory until final write - Combines template Write of various types into a single write which uses the filesystem of the applicable templates's repository - Template is now a simple noun, with logic localized to the templates manager - Adds better error message regarding single repo mode conditions The reason for replacing filesystem-based repository instances with in-memory (especially remote for WithRepository mode) is for a few reasons: First, 'Add' will fail without using the temporary UUID hack if there already exists a repository of the _repo_ name (due to a filesystem collision). We have to read the repo to determine the default name as well, so it's messy to avoid. Second, using an on-disk version is potentially leaky because it may leave files on the system in the event of a process interruption; able to be mitigated, but with a likewise hacky use of a os temp directory. Third, it may cause racing conditions when using multiple instances the Client as a lib. Lastly, it precludes our ability to run without touching the filesystm: a useful mode when running as a pure library or in security-restricted environments. * fix: paths within embedded fs * src: repositories code cleanup * Removes the manual 0_18 version suffix * Reanames "single" (uri for single-mode) to the more descriptive "remote" (with associated accessor api) * Default repo name when no URI provided is set to default * Various wording improvements in error text and comments * src: error formatting * feat: templates from local file paths Adds back the local os filesystem as a fallthrough to support loading template repositories from file:// paths on disk, without the requirement that they also be git repositories. * fix: clone once Fixes the shortcut of performing a clone to read the manifest prior to cloning the repository to disk. This double-clone was replaced by a single clone (applying the manifest) followed by a file copy from the in-memory FS to the on-disk repository location. * src: test fixes * respect explict name on repository .Add * fail if repository already exists * update expected test filenames to new general (non-go-specific) setup * adds the default repo name to the NewRepository constructor, internalizing the somewhat tricky default logic. * move repo write logig to repo itself, as it will contain impl-specific nuances. * invalid repo path is an error, but only if defined * temporarily disable repository URL test * src: update templates test to reflect new name 'cloudevents' * src: fix go templates and repackage * fixup: reviewdog spelling errors Signed-off-by: Lance Ball <lball@redhat.com> * fixup: reviewdog whitespace cleanup Signed-off-by: Lance Ball <lball@redhat.com> * fixup: run ./hack/update-codegen.sh Signed-off-by: Lance Ball <lball@redhat.com> * fixup: restore boson-project templates for e2e Signed-off-by: Lance Ball <lball@redhat.com> * fixup: do not error if default repo location does not exist Signed-off-by: Lance Ball <lball@redhat.com> * fixup: typo Signed-off-by: Lance Ball <lball@redhat.com> * paketo builder and paket community rust buildpack (#599) * fixup: apply f161d50 Signed-off-by: Lance Ball <lball@redhat.com> * fixup: update documentation to match impl Signed-off-by: Lance Ball <lball@redhat.com> * fixup: add git attributes to ignore linting on binary files Signed-off-by: Lance Ball <lball@redhat.com> * fixup: remove whitespace Signed-off-by: Lance Ball <lball@redhat.com> * fixup: tweak linter rules Signed-off-by: Lance Ball <lball@redhat.com> * fixup: tweak .gitattributes Signed-off-by: Lance Ball <lball@redhat.com> Co-authored-by: Luke Kingland <lkingland2038@gmail.com> Co-authored-by: Shashankft9 <48708039+Shashankft9@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Vendors providing new language packs should know how to structure the directory
Current behavior
Currently, the
func
CLI assumes that all top level directories in a language pack repository represent a runtime, and each subdirectory is a template directory.For example, when installing the https://github.com/lance/gcf-kn language pack repository, the ruby cloud-function template would be in
~/.config/func/repositories/gcf-kn/ruby/cloud-function
. The CLI can examine the root directory for the repository (gcf-kn
), and iterate through all of the sub directories in order to display a list of available runtimes and templates.However, this is a fairly rudimentary way to handle it. It could be common, for example, that vendors want to include the buildpacks for the language pack in this same directory. Where do they put that? Will that directory then be listed as an available runtime?
A strawman proposal
We can resolve this by publishing a specification for a language pack's directory structure.
kn func create
Acceptance criteria
The text was updated successfully, but these errors were encountered: