Skip to content

Commit

Permalink
Feature/enable tags (#1073)
Browse files Browse the repository at this point in the history
* Add initial Exacloud page to documentation

* Update Exacloud support

* Clean up console debug output

[no ci]

* Add default Worker engine ('docker')

[no ci]

* Update configurable Container Engine commands

* Revert changes to docker implementation and add engine interface

[no ci]

* Update Exacloud/Container Engine implementation

[no ci]

* Update ContainerEngine interface

[no ci]

* Rename ContainerEngine to ContainerType

* Add initial Exadocker unit test

[no ci]

* Update TaskService to pass config to Funnel worker

[no ci]

* Add small fix in HPC Template config

[no ci]

* Update worker config path in HPC Template

[no ci]

* Add debugging from Exacloud testing

* Update input/output working example (md5sum)

- Test command:
```sh
funnel task create examples/md5sum.json
```

[no ci]

* Update docker.go to implement ContainerEngine interface

- Can be used in HPC environment with unique docker setups (e.g. exadocker) as well as future command templating efforts.

[no ci]

* Remove exadocker implementation in favor of docker

* Add initial HPC documentation and example

* Update docker and kin-openapi to compatible versions

- github.com/docker/docker versions above v24.0.9 resulted in `panic: Can't connect docker client`:

```
➜ go test tests/auth/auth_test.go
--- FAIL: TestBasicAuthFail (0.01s)
panic: Can't connect docker client [recovered]
        panic: Can't connect docker client

FAIL    command-line-arguments  0.673s
FAIL
```

- This error appears to be originating from [util/dockerutil/docker.go](https://github.com/ohsu-comp-bio/funnel/blob/f5acbabe85d28f45f2f86db959c4d7bc929334a4/util/dockerutil/docker.go#L28).

- github.com/getkin/kin-openapi versions above v0.112.0 resulted in type errors:

v0.113.0:
```
➜ go test util/openapi2proto/main.go

util/openapi2proto/main.go:54:38: invalid operation: p.Value.AdditionalProperties != nil (mismatched types openapi3.AdditionalProperties and untyped nil)
util/openapi2proto/main.go:55:24: cannot use p.Value.AdditionalProperties (variable of type openapi3.AdditionalProperties) as *openapi3.SchemaRef value in argument to getType
FAIL    command-line-arguments [build failed]
FAIL
```

v0.123.0 (latest):
```
➜ go test util/openapi2proto/main.go

util/openapi2proto/main.go:43:7: cannot convert "integer" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:45:7: cannot convert "boolean" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:47:7: cannot convert "number" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:49:7: cannot convert "object" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:54:38: invalid operation: p.Value.AdditionalProperties != nil (mismatched types openapi3.AdditionalProperties and untyped nil)
util/openapi2proto/main.go:55:24: cannot use p.Value.AdditionalProperties (variable of type openapi3.AdditionalProperties) as *openapi3.SchemaRef value in argument to getType
util/openapi2proto/main.go:60:7: cannot convert "array" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:72:17: cannot use p.Value.Type (variable of type *openapi3.Types) as string value in return statement
util/openapi2proto/main.go:84:32: invalid operation: param.Schema.Value.Type != "" (mismatched types *openapi3.Types and untyped string)
util/openapi2proto/main.go:175:17: resp.Get undefined (type *openapi3.Responses has no field or method Get)
util/openapi2proto/main.go:175:17: too many errors
FAIL    command-line-arguments [build failed]
FAIL
```

* Update util/openapi2proto/main.go with latest kin-openapi version

* Update github.com/docker/docker to latest version

* Update Docker client creation in tests

* Add `WithAPIVersionNegotiation` to docker client creation

* Rename 'Driver' to 'DriverCommand' in ContainerConfig

* Fix minor linting errors

* Update GitHub Actions workflow (linting)

* fix: updated config check in hpc_backend.go

* Revert tabs/space change in storage/local.go

* Update output copying in scratch directory

* Update compliance-test.yaml

* Debug: add tmate step to Nextflow test

* Update Nextflow tests

* Update file copying behavior for Nextflow tests

* Update local file copying behavior to support directory dests

* Update local dest for files with glob patterns

* Update Base64Encode test to match Github Actions test

- TODO: investigate why this is necessary...

* Fix Base64Encoding test value

* Revert glob output change based on TesOutput spec

- Wildcards in outputs will be uploaded to a directory, with no special handling for the filename.

```yaml
tesOutput:
  properties:
    url:
     description: |-
        URL at which the TES server makes the output accessible after the task is complete. When tesOutput.path contains wildcards, it must be a directory; see `tesOutput.path_prefix` for details on how output URLs are constructed in this case.
```

* Debug: Add tmate step to Nextflow tests

* Update wildcard file output destination

* Add initial command templating for container engines

* Add initial Docker/ContainerEngine tests

* Run `go mod tidy`

* Update Github Actions

* Update container engine IO and command execution

* Re-enable all tests for container engines

* Update test suite

* Nextflow test debug

* Nextflow test debug

* Nextflow test debug

* Nextflow test debug

* Update compliance test workflow

* Update S3 tests

* Update S3 tests

* Add comments and clean up debug statements

* Clean up code for 0.11.1-rc.1 release

* Add container command event to match existing pattern

* Update .goreleaser.yaml to replace deprecated fields

* Update Nextflow instructions

* Update K8s support and documentation

- Update docker images to use quay.io/ohsu-comp-bio/funnel

* Update README.md

* fix: add support for wildcards in AWS S3 paths

* Add support for wildcards in AWS S3 output paths

* Update actions/upload-artifact to v4 in Github Actions

-
https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

* Update version to 0.11.1-rc.4 in Makefile

* Fix bug where FileMapper output was being overwritten in every loop

* Add initial Exacloud page to documentation

* Update Exacloud support

* Clean up console debug output

[no ci]

* Add default Worker engine ('docker')

[no ci]

* Update configurable Container Engine commands

* Revert changes to docker implementation and add engine interface

[no ci]

* Update Exacloud/Container Engine implementation

[no ci]

* Update ContainerEngine interface

[no ci]

* Rename ContainerEngine to ContainerType

* Add initial Exadocker unit test

[no ci]

* Update TaskService to pass config to Funnel worker

[no ci]

* Add small fix in HPC Template config

[no ci]

* Update worker config path in HPC Template

[no ci]

* Add debugging from Exacloud testing

* Update input/output working example (md5sum)

- Test command:
```sh
funnel task create examples/md5sum.json
```

[no ci]

* Update docker.go to implement ContainerEngine interface

- Can be used in HPC environment with unique docker setups (e.g. exadocker) as well as future command templating efforts.

[no ci]

* Remove exadocker implementation in favor of docker

* Add initial HPC documentation and example

* Update docker and kin-openapi to compatible versions

- github.com/docker/docker versions above v24.0.9 resulted in `panic: Can't connect docker client`:

```
➜ go test tests/auth/auth_test.go
--- FAIL: TestBasicAuthFail (0.01s)
panic: Can't connect docker client [recovered]
        panic: Can't connect docker client

FAIL    command-line-arguments  0.673s
FAIL
```

- This error appears to be originating from [util/dockerutil/docker.go](https://github.com/ohsu-comp-bio/funnel/blob/f5acbabe85d28f45f2f86db959c4d7bc929334a4/util/dockerutil/docker.go#L28).

- github.com/getkin/kin-openapi versions above v0.112.0 resulted in type errors:

v0.113.0:
```
➜ go test util/openapi2proto/main.go

util/openapi2proto/main.go:54:38: invalid operation: p.Value.AdditionalProperties != nil (mismatched types openapi3.AdditionalProperties and untyped nil)
util/openapi2proto/main.go:55:24: cannot use p.Value.AdditionalProperties (variable of type openapi3.AdditionalProperties) as *openapi3.SchemaRef value in argument to getType
FAIL    command-line-arguments [build failed]
FAIL
```

v0.123.0 (latest):
```
➜ go test util/openapi2proto/main.go

util/openapi2proto/main.go:43:7: cannot convert "integer" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:45:7: cannot convert "boolean" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:47:7: cannot convert "number" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:49:7: cannot convert "object" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:54:38: invalid operation: p.Value.AdditionalProperties != nil (mismatched types openapi3.AdditionalProperties and untyped nil)
util/openapi2proto/main.go:55:24: cannot use p.Value.AdditionalProperties (variable of type openapi3.AdditionalProperties) as *openapi3.SchemaRef value in argument to getType
util/openapi2proto/main.go:60:7: cannot convert "array" (untyped string constant) to type *openapi3.Types
util/openapi2proto/main.go:72:17: cannot use p.Value.Type (variable of type *openapi3.Types) as string value in return statement
util/openapi2proto/main.go:84:32: invalid operation: param.Schema.Value.Type != "" (mismatched types *openapi3.Types and untyped string)
util/openapi2proto/main.go:175:17: resp.Get undefined (type *openapi3.Responses has no field or method Get)
util/openapi2proto/main.go:175:17: too many errors
FAIL    command-line-arguments [build failed]
FAIL
```

* Update util/openapi2proto/main.go with latest kin-openapi version

* Update github.com/docker/docker to latest version

* Update Docker client creation in tests

* Add `WithAPIVersionNegotiation` to docker client creation

* Rename 'Driver' to 'DriverCommand' in ContainerConfig

* Fix minor linting errors

* Update GitHub Actions workflow (linting)

* fix: updated config check in hpc_backend.go

* Revert tabs/space change in storage/local.go

* Update output copying in scratch directory

* Update compliance-test.yaml

* Debug: add tmate step to Nextflow test

* Update Nextflow tests

* Update file copying behavior for Nextflow tests

* Update local file copying behavior to support directory dests

* Update local dest for files with glob patterns

* Update Base64Encode test to match Github Actions test

- TODO: investigate why this is necessary...

* Fix Base64Encoding test value

* Revert glob output change based on TesOutput spec

- Wildcards in outputs will be uploaded to a directory, with no special handling for the filename.

```yaml
tesOutput:
  properties:
    url:
     description: |-
        URL at which the TES server makes the output accessible after the task is complete. When tesOutput.path contains wildcards, it must be a directory; see `tesOutput.path_prefix` for details on how output URLs are constructed in this case.
```

* Debug: Add tmate step to Nextflow tests

* Update wildcard file output destination

* Add initial command templating for container engines

* Add initial Docker/ContainerEngine tests

* Run `go mod tidy`

* Update Github Actions

* Update container engine IO and command execution

* Re-enable all tests for container engines

* Update test suite

* Nextflow test debug

* Nextflow test debug

* Nextflow test debug

* Nextflow test debug

* Update compliance test workflow

* Update S3 tests

* Update S3 tests

* Add comments and clean up debug statements

* Clean up code for 0.11.1-rc.1 release

* Add container command event to match existing pattern

* Update .goreleaser.yaml to replace deprecated fields

* Update Nextflow instructions

* Update K8s support and documentation

- Update docker images to use quay.io/ohsu-comp-bio/funnel

* Update README.md

* fix: add support for wildcards in AWS S3 paths

* Add support for wildcards in AWS S3 output paths

* Update actions/upload-artifact to v4 in Github Actions

-
https://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/

* Update version to 0.11.1-rc.4 in Makefile

* Fix bug where FileMapper output was being overwritten in every loop

* Add initial search functionality to website

- Uses Pagefind: https://pagefind.app/docs/

* Update GitHub Actions workflows

* Add Pagefind step to hugo.yaml

* Run `go mod tidy`

* Fix lint errors

* debug: Slurm Github Action workflow

* debug: Slurm GitHub Actions Workflow

* debug: Slurm GitHub Actions Workflow

* debug: Slurm GitHub Actions Workflow

* debug: Slurm GitHub Actions Workflow

* debug: Slurm GitHub Actions Workflow

* Add initial support for tag propagation (thank you @robertbio!)

- Adapted for latest container engine updates, originally from: #709
  • Loading branch information
lbeckman314 authored Oct 3, 2024
1 parent 8166db7 commit ce59261
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 8 deletions.
2 changes: 2 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ type ContainerConfig struct {
RunCommand string // template string
PullCommand string // template string
StopCommand string // template string
EnableTags bool
Tags map[string]string
}

// HPCBackend describes the configuration for a HPC scheduler backend such as
Expand Down
27 changes: 19 additions & 8 deletions config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,30 @@ func DefaultConfig() Config {
LogUpdateRate: Duration(time.Second * 5),
LogTailSize: 10000,
MaxParallelTransfers: 10,
// `docker run` command flags
// https://docs.docker.com/reference/cli/docker/container/run/
Container: ContainerConfig{
DriverCommand: "docker",
RunCommand: "run -i --read-only " +
// Remove container after it exits
"{{if .RemoveContainer}}--rm{{end}} " +
"{{range $k, $v := .Env}}-e {{$k}}={{$v}} {{end}} " +

// Environment variables
"{{range $k, $v := .Env}}--env {{$k}}={{$v}} {{end}} " +

// Tags/Labels
"{{range $k, $v := .Tags}}--label {{$k}}={{$v}} {{end}} " +

// Container Name
"{{if .Name}}--name {{.Name}}{{end}} " +
"{{if .Workdir}}-w {{.Workdir}}{{end}} " +
"{{range .Volumes}}-v {{.HostPath}}:{{.ContainerPath}}:{{if .Readonly}}ro{{else}}rw{{end}} {{end}} " +

// Workdir
"{{if .Workdir}}--workdir {{.Workdir}}{{end}} " +

// Volumes
"{{range .Volumes}}--volume {{.HostPath}}:{{.ContainerPath}}:{{if .Readonly}}ro{{else}}rw{{end}} {{end}} " +

// Image and Command
"{{.Image}} {{.Command}}",
PullCommand: "pull {{.Image}}",
StopCommand: "rm -f {{.Name}}",
Expand Down Expand Up @@ -146,11 +162,6 @@ func DefaultConfig() Config {
c.AWSBatch.ReconcileRate = reconcile
c.AWSBatch.DisableReconciler = true

kubernetesTemplate := intern.MustAsset("config/kubernetes-template.yaml")
executorTemplate := intern.MustAsset("config/kubernetes-executor-template.yaml")
c.Kubernetes.Executor = "docker"
c.Kubernetes.Template = string(kubernetesTemplate)
c.Kubernetes.ExecutorTemplate = string(executorTemplate)
c.Kubernetes.ReconcileRate = reconcile

return c
Expand Down
7 changes: 7 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ require (
github.com/go-openapi/spec v0.21.0 // indirect
github.com/go-openapi/strfmt v0.23.0 // indirect
github.com/go-openapi/swag v0.23.0 // indirect
github.com/go-openapi/validate v0.24.0 // indirect
github.com/go-restruct/restruct v1.2.0-alpha // indirect
github.com/go-telegram-bot-api/telegram-bot-api/v5 v5.5.1 // indirect
github.com/gobwas/glob v0.2.3 // indirect
github.com/golang-jwt/jwt/v4 v4.5.0 // indirect
github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
github.com/golang/glog v1.2.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/snappy v0.0.4 // indirect
Expand All @@ -205,6 +211,7 @@ require (
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/s2a-go v0.1.8 // indirect
github.com/google/uuid v1.6.0 // indirect
github.com/google/wire v0.6.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect
github.com/googleapis/gax-go/v2 v2.13.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions worker/container_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ type ContainerConfig struct {
RunCommand string // template string
PullCommand string // template string
StopCommand string // template string
EnableTags bool
Tags map[string]string
}

type ContainerVersion struct {
Expand Down
20 changes: 20 additions & 0 deletions worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"os"
"path/filepath"
"regexp"
"time"

"github.com/ohsu-comp-bio/funnel/config"
Expand Down Expand Up @@ -174,6 +175,8 @@ func (r *DefaultWorker) Run(pctx context.Context) (runerr error) {
containerConfig := ContainerConfig{
Image: d.Image,
Command: d.Command,
// TODO: Where is d.Env set?
// Do we need to sanitize these values as well?
Env: d.Env,
Volumes: mapper.Volumes,
Workdir: d.Workdir,
Expand All @@ -186,6 +189,15 @@ func (r *DefaultWorker) Run(pctx context.Context) (runerr error) {
containerConfig.RunCommand = r.Conf.Container.RunCommand
containerConfig.PullCommand = r.Conf.Container.PullCommand
containerConfig.StopCommand = r.Conf.Container.StopCommand

// Hide this behind explicit flag/option in configuration
if r.Conf.Container.EnableTags {
for k, v := range task.Tags {
safeTag := r.sanitizeValues(v)
containerConfig.Tags[k] = safeTag
}
}

containerEngine, err := f.NewContainerEngine(containerConfig)
if err != nil {
run.syserr = err
Expand Down Expand Up @@ -307,6 +319,14 @@ func (r *DefaultWorker) validate(mapper *FileMapper) error {
return nil
}

// Sanitizes the input string to avoid command injection.
// Only allows alphanumeric characters, dashes, underscores, and dots.
func (r *DefaultWorker) sanitizeValues(value string) string {
// Replace anything that is not an alphanumeric character, dash, underscore, or dot
re := regexp.MustCompile(`[^a-zA-Z0-9-_\.]`)
return re.ReplaceAllString(value, "")
}

func (r *DefaultWorker) pollForCancel(pctx context.Context, cancelCallback func()) context.Context {
taskctx, cancel := context.WithCancel(pctx)

Expand Down

0 comments on commit ce59261

Please sign in to comment.