diff --git a/docs/runware_serverless_deploy.md b/docs/runware_serverless_deploy.md index 42c7efe..82e6a59 100644 --- a/docs/runware_serverless_deploy.md +++ b/docs/runware_serverless_deploy.md @@ -6,9 +6,32 @@ Deploy a new serverless application Create a new serverless application from a Python entry file. -The file is zipped and submitted as the application source. Worker settings -are supplied via flags (a local project config via 'runware serverless init' -is planned). Endpoints are derived server-side from the SDK. +The whole source directory is zipped and submitted as the application source, so +the entry file can import its own modules and read its own data files. That +directory is the working directory unless --src-dir says otherwise. + +The entry file must live inside the source directory. A relative path is resolved +inside it; an absolute path is taken as given. + +Exclude what the app does not need with a .runwareignore file at the root of the +source directory; it takes gitignore syntax. A .gitignore is NOT consulted -- +what a project keeps out of version control is a different question from what it +ships. Either way .env files are never uploaded, and neither are .git, +__pycache__, .venv, node_modules or the usual build and tool caches. + +Environment variables must be supplied here with --env or --env-file. An app's +environment is frozen into the version this command creates, which is what the +worker is rendered from, so setting one afterwards with 'apps env set' stores it +without it ever reaching a pod. Prefer --env-file for anything secret: a value +passed as --env is visible in the process list and recorded in shell history. + +Anything the app downloads at runtime belongs on a --volume. The app runs in a +sandbox whose filesystem is part of the checkpointed state, so an unmounted +download is copied into every checkpoint and fetched again on every cold start. +A volume keeps it out of both. + +Worker settings are supplied via flags (a local project config via 'runware +serverless init' is planned). Endpoints are derived server-side from the SDK. ``` runware serverless deploy [flags] @@ -17,9 +40,23 @@ runware serverless deploy [flags] ### Examples ``` - # deploy a Python entry file + # deploy the current directory, with app.py as the entry point runware serverless deploy ./app.py --id my-app --gpu-type h100 + # deploy a project that lives elsewhere; app.py is resolved inside --src-dir + runware serverless deploy app.py --src-dir ~/projects/my-app --id my-app --gpu-type h100 + + # an entry file in a subdirectory of the project + runware serverless deploy src/app.py --src-dir ~/projects/my-app --id my-app --gpu-type h100 + + # pass a token to the worker without putting it in the process list + printf 'HF_TOKEN=%s' "$token" > .env.deploy + runware serverless deploy model.py --id my-app --gpu-type l40s --env-file .env.deploy + + # keep downloaded model weights on persistent node-local storage + runware serverless deploy model.py --id my-app --gpu-type l40s \ + --volume /root/.cache/huggingface + # override worker settings and base image runware serverless deploy ./app.py --id my-app --name "My App" \ --max-workers 2 --idle-ttl 120 --gpu-type h100 \ @@ -30,6 +67,8 @@ runware serverless deploy [flags] ``` --base-image string Builder base image (default "python:3.11-slim") + --env stringArray Environment variable as KEY=VALUE (repeatable) + --env-file stringArray File of KEY=VALUE lines to read environment variables from (repeatable) --gpu-type string GPU type ID (see 'serverless gpus') --gpus-per-worker int32 GPUs allocated per worker (default 1) -h, --help help for deploy @@ -40,6 +79,8 @@ runware serverless deploy [flags] --name string Display name (defaults to --id) --requirement stringArray Additional pip package to install (repeatable) --scaling-delay int32 Scaling delay in seconds (default 10) + --src-dir string Directory to package as the application source (default: the working directory) + --volume stringArray Absolute path inside the app backed by persistent node-local storage (repeatable) ``` ### Options inherited from parent commands diff --git a/go.mod b/go.mod index 24bd7dd..ddfc50c 100644 --- a/go.mod +++ b/go.mod @@ -5,13 +5,15 @@ go 1.26.4 require ( github.com/briandowns/spinner v1.23.2 github.com/charmbracelet/log v1.0.0 + github.com/go-git/go-git/v5 v5.16.3 github.com/google/uuid v1.6.0 github.com/gorilla/websocket v1.5.3 github.com/jedib0t/go-pretty/v6 v6.7.10 github.com/oapi-codegen/runtime v1.6.0 github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c github.com/spf13/cobra v1.10.2 - golang.org/x/term v0.41.0 + github.com/spf13/pflag v1.0.10 + golang.org/x/term v0.45.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -28,10 +30,13 @@ require ( github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect github.com/fatih/color v1.7.0 // indirect github.com/getkin/kin-openapi v0.142.0 // indirect + github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect + github.com/go-git/go-billy/v5 v5.6.2 // indirect github.com/go-logfmt/logfmt v0.6.1 // indirect github.com/go-openapi/jsonpointer v0.23.1 // indirect github.com/go-openapi/swag/jsonname v0.26.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-colorable v0.1.14 // indirect github.com/mattn/go-isatty v0.0.20 // indirect @@ -45,16 +50,17 @@ require ( github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect github.com/speakeasy-api/jsonpath v0.6.3 // indirect github.com/speakeasy-api/openapi v1.24.0 // indirect - github.com/spf13/pflag v1.0.10 // indirect github.com/vmware-labs/yaml-jsonpath v0.3.2 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/mod v0.38.0 // indirect + golang.org/x/net v0.57.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/text v0.40.0 // indirect golang.org/x/tools v0.48.0 // indirect + gopkg.in/warnings.v0 v0.1.2 // indirect ) tool github.com/oapi-codegen/oapi-codegen/v2/cmd/oapi-codegen diff --git a/go.sum b/go.sum index ac91484..4e0d51b 100644 --- a/go.sum +++ b/go.sum @@ -41,6 +41,12 @@ github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWo github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/getkin/kin-openapi v0.142.0 h1:izj0vBdFprMhitfzaX8sTqztsEQyvwhssBoB6n8NO7w= github.com/getkin/kin-openapi v0.142.0/go.mod h1:3BH9M9XDe/y9M5DSvEocVYAYq1w0qrhJHjC/vZi0AaY= +github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 h1:+zs/tPmkDkHx3U66DAb0lQFJrpS6731Oaa12ikc+DiI= +github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376/go.mod h1:an3vInlBmSxCcxctByoQdvwPiA7DTK7jaaFDBTtu0ic= +github.com/go-git/go-billy/v5 v5.6.2 h1:6Q86EsPXMa7c3YZ3aLAQsMA0VlWmy43r6FHqa/UNbRM= +github.com/go-git/go-billy/v5 v5.6.2/go.mod h1:rcFC2rAsp/erv7CMz9GczHcuD0D32fWzH+MJAU+jaUU= +github.com/go-git/go-git/v5 v5.16.3 h1:Z8BtvxZ09bYm/yYNgPKCzgWtaRqDTgIKRgIRHBfU6Z8= +github.com/go-git/go-git/v5 v5.16.3/go.mod h1:4Ge4alE/5gPs30F2H1esi2gPd69R0C39lolkucHBOp8= github.com/go-logfmt/logfmt v0.6.1 h1:4hvbpePJKnIzH1B+8OR/JPbTx37NktoI9LE2QZBBkvE= github.com/go-logfmt/logfmt v0.6.1/go.mod h1:EV2pOAQoZaT1ZXZbqDl5hrymndi4SY9ED9/z6CO0XAk= github.com/go-openapi/jsonpointer v0.23.1 h1:1HBACs7XIwR2RcmItfdSFlALhGbe6S92p0ry4d1GWg4= @@ -74,6 +80,8 @@ github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpO github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A= +github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo= github.com/jedib0t/go-pretty/v6 v6.7.10 h1:B/2qW2Bkv2L6n14PP8o1kx75kWzHOQ3YTluWzg9icac= github.com/jedib0t/go-pretty/v6 v6.7.10/go.mod h1:YwC5CE4fJ1HFUDeivSV1r//AmANFHyqczZk+U6BDALU= github.com/juju/gnuflag v0.0.0-20171113085948-2ce1bb71843d/go.mod h1:2PavIy+JPciBPrBUjwbNvtwB6RQlve+hkpll6QSNmOE= @@ -117,10 +125,13 @@ github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1Cpa github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= -github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= +github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k= +github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c h1:+mdjkGKdHQG3305AYmdv1U2eRNDiU2ErMBj1gwrq8eQ= github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c/go.mod h1:7rwL4CYBLnjLxUqIJNnCWiEdr3bn6IUYi15bNlnbCCU= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -132,8 +143,9 @@ github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= -github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= +github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8= +github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= github.com/speakeasy-api/jsonpath v0.6.3 h1:c+QPwzAOdrWvzycuc9HFsIZcxKIaWcNpC+xhOW9rJxU= github.com/speakeasy-api/jsonpath v0.6.3/go.mod h1:2cXloNuQ+RSXi5HTRaeBh7JEmjRXTiaKpFTdZiL7URI= github.com/speakeasy-api/openapi v1.24.0 h1:opoD27rupX7zBVPq1HkIGLeMOzNNA7JalhYP8q34i04= @@ -160,8 +172,8 @@ go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0 h1:985EYyeCOxTpcgOTJpflJUwOeEz0CQOdPt73OzpE9F8= -golang.org/x/exp v0.0.0-20240404231335-c0f41cb1a7a0/go.mod h1:/lliqkxwWAhPjf5oSOIJup2XcqJaw8RGS6k3TGEc7GI= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8= +golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= @@ -199,8 +211,8 @@ golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= -golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -231,11 +243,12 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EV gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= +gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= +gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/api/serverless/client.go b/internal/api/serverless/client.go index 4638269..23eda55 100644 --- a/internal/api/serverless/client.go +++ b/internal/api/serverless/client.go @@ -46,6 +46,9 @@ type CodeSourceUpsert = gen.CodeSourceUpsert // CodebaseSource is the zipped customer code payload. type CodebaseSource = gen.CodebaseSource +// AppVolume is a persistent node-local directory mounted into the application. +type AppVolume = gen.AppVolume + // WorkerConfig is the live worker configuration on an app. type WorkerConfig = gen.WorkerConfig diff --git a/internal/cmd/serverless/deploy.go b/internal/cmd/serverless/deploy.go index 3f5acdd..69bf5c4 100644 --- a/internal/cmd/serverless/deploy.go +++ b/internal/cmd/serverless/deploy.go @@ -24,6 +24,10 @@ func newDeployCmd(logger *log.Logger) *cobra.Command { requirements []string minWorkers int32 gpusPerWorker int32 + srcDir string + volumes []string + envVars []string + envFiles []string ) cmd := &cobra.Command{ @@ -31,12 +35,49 @@ func newDeployCmd(logger *log.Logger) *cobra.Command { Short: "Deploy a new serverless application", Long: `Create a new serverless application from a Python entry file. -The file is zipped and submitted as the application source. Worker settings -are supplied via flags (a local project config via 'runware serverless init' -is planned). Endpoints are derived server-side from the SDK.`, - Example: ` # deploy a Python entry file +The whole source directory is zipped and submitted as the application source, so +the entry file can import its own modules and read its own data files. That +directory is the working directory unless --src-dir says otherwise. + +The entry file must live inside the source directory. A relative path is resolved +inside it; an absolute path is taken as given. + +Exclude what the app does not need with a .runwareignore file at the root of the +source directory; it takes gitignore syntax. A .gitignore is NOT consulted -- +what a project keeps out of version control is a different question from what it +ships. Either way .env files are never uploaded, and neither are .git, +__pycache__, .venv, node_modules or the usual build and tool caches. + +Environment variables must be supplied here with --env or --env-file. An app's +environment is frozen into the version this command creates, which is what the +worker is rendered from, so setting one afterwards with 'apps env set' stores it +without it ever reaching a pod. Prefer --env-file for anything secret: a value +passed as --env is visible in the process list and recorded in shell history. + +Anything the app downloads at runtime belongs on a --volume. The app runs in a +sandbox whose filesystem is part of the checkpointed state, so an unmounted +download is copied into every checkpoint and fetched again on every cold start. +A volume keeps it out of both. + +Worker settings are supplied via flags (a local project config via 'runware +serverless init' is planned). Endpoints are derived server-side from the SDK.`, + Example: ` # deploy the current directory, with app.py as the entry point runware serverless deploy ./app.py --id my-app --gpu-type h100 + # deploy a project that lives elsewhere; app.py is resolved inside --src-dir + runware serverless deploy app.py --src-dir ~/projects/my-app --id my-app --gpu-type h100 + + # an entry file in a subdirectory of the project + runware serverless deploy src/app.py --src-dir ~/projects/my-app --id my-app --gpu-type h100 + + # pass a token to the worker without putting it in the process list + printf 'HF_TOKEN=%s' "$token" > .env.deploy + runware serverless deploy model.py --id my-app --gpu-type l40s --env-file .env.deploy + + # keep downloaded model weights on persistent node-local storage + runware serverless deploy model.py --id my-app --gpu-type l40s \ + --volume /root/.cache/huggingface + # override worker settings and base image runware serverless deploy ./app.py --id my-app --name "My App" \ --max-workers 2 --idle-ttl 120 --gpu-type h100 \ @@ -48,7 +89,17 @@ is planned). Endpoints are derived server-side from the SDK.`, name = id } - zipBase64, modelFile, err := packPythonFile(entryFile) + zipBase64, modelFile, err := packDirectory(srcDir, entryFile) + if err != nil { + return err + } + + appVolumes, err := buildVolumes(volumes) + if err != nil { + return err + } + + appEnv, err := buildEnvironmentVariables(envFiles, envVars) if err != nil { return err } @@ -66,9 +117,11 @@ is planned). Endpoints are derived server-side from the SDK.`, } body := serverlessapi.AppCreate{ - AppId: id, - AppName: name, - AppSource: source, + AppId: id, + AppName: name, + AppSource: source, + Volumes: appVolumes, + EnvironmentVariables: appEnv, Configuration: serverlessapi.WorkerConfigCreate{ MaxWorkers: maxWorkers, IdleTtlSecs: idleTTL, @@ -94,6 +147,10 @@ is planned). Endpoints are derived server-side from the SDK.`, }, } + cmd.Flags().StringVar(&srcDir, "src-dir", "", "Directory to package as the application source (default: the working directory)") + cmd.Flags().StringArrayVar(&volumes, "volume", nil, "Absolute path inside the app backed by persistent node-local storage (repeatable)") + cmd.Flags().StringArrayVar(&envVars, "env", nil, "Environment variable as KEY=VALUE (repeatable)") + cmd.Flags().StringArrayVar(&envFiles, "env-file", nil, "File of KEY=VALUE lines to read environment variables from (repeatable)") cmd.Flags().StringVar(&id, "id", "", "Application ID (immutable, lowercase slug)") cmd.Flags().StringVar(&name, "name", "", "Display name (defaults to --id)") cmd.Flags().Int32Var(&maxWorkers, "max-workers", 1, "Maximum number of workers") diff --git a/internal/cmd/serverless/apps_env.go b/internal/cmd/serverless/env.go similarity index 50% rename from internal/cmd/serverless/apps_env.go rename to internal/cmd/serverless/env.go index ed190da..d0566a0 100644 --- a/internal/cmd/serverless/apps_env.go +++ b/internal/cmd/serverless/env.go @@ -3,6 +3,10 @@ package serverless import ( "fmt" "log/slog" + "os" + "regexp" + "strings" + "unicode/utf8" "github.com/charmbracelet/log" serverlessapi "github.com/runware/runware-cli/internal/api/serverless" @@ -163,3 +167,139 @@ func newAppsEnvUnsetCmd(logger *log.Logger) *cobra.Command { }, } } + +// --------------------------------------------------------------------------- +// Create-time environment variables, for `deploy --env` / `--env-file`. +// --------------------------------------------------------------------------- + +// Environment variable limits, mirrored from the server's EnvironmentVariableName +// and its deployment_configs column CHECK. +const ( + maxEnvVars = 100 + maxEnvNameLen = 128 + maxEnvValueLen = 4096 + envAssignSuffix = "=VALUE" +) + +// envNamePattern is the server's EnvironmentVariableName rule: POSIX-style, so a +// name this accepts is a name the API can store rather than one it rejects after +// the archive has already been uploaded. +var envNamePattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]{0,127}$`) + +// buildEnvironmentVariables turns --env KEY=VALUE pairs and --env-file paths into +// the create request's map. +// +// These belong on the CREATE request and nowhere else: an app's environment is +// frozen into its version snapshot, which is what the deployer renders from, and +// no endpoint creates a further version -- `deploy` re-applies an existing one by +// number and says so. So a variable set through the /environment-variables +// endpoints after the app exists is stored, listed back, and never reaches a +// worker. Passing it here is the only route that ends up in a pod. +// +// Files are read before the inline pairs are applied, so an explicit --env wins +// over a file entry with the same name. +func buildEnvironmentVariables(files, pairs []string) (*map[string]string, error) { + if len(files) == 0 && len(pairs) == 0 { + return nil, nil + } + + env := make(map[string]string) + for _, path := range files { + if err := readEnvFile(path, env); err != nil { + return nil, err + } + } + for _, pair := range pairs { + name, value, err := splitEnvAssignment(pair) + if err != nil { + return nil, err + } + env[name] = value + } + + if len(env) > maxEnvVars { + return nil, fmt.Errorf("at most %d environment variables (got %d)", maxEnvVars, len(env)) + } + return &env, nil +} + +// readEnvFile reads KEY=VALUE lines into env, skipping blanks and # comments. +func readEnvFile(path string, env map[string]string) error { + raw, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("read env file: %w", err) + } + for i, line := range strings.Split(string(raw), "\n") { + // The trimmed copy decides whether the line carries an assignment at all; + // the assignment itself is parsed from the original, because trailing + // whitespace in a value can be deliberate and this is not the place to + // silently rewrite it. + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + assignment := strings.TrimSuffix(line, "\r") + // `export FOO=bar` is what a shell-sourced file looks like, and pasting + // one in is the obvious mistake to absorb rather than reject. Trimmed on + // the left only, so the value keeps whatever follows the `=`. + assignment = strings.TrimPrefix(strings.TrimLeft(assignment, " \t"), "export ") + name, value, err := splitEnvAssignment(assignment) + if err != nil { + return fmt.Errorf("%s line %d: %w", path, i+1, err) + } + env[name] = value + } + return nil +} + +// unquote strips one matching pair of surrounding quotes. +// +// A .env file written by hand or produced by another tool routinely quotes +// values, and shells strip those quotes when sourcing the file. Passing them +// through would send `"hf_x"` as the token itself -- a 401 inside the pod with +// nothing in the logs to explain it. One pair only, and only when it matches, so +// a value that genuinely contains a quote keeps it. +func unquote(value string) string { + if len(value) < 2 { + return value + } + first, last := value[0], value[len(value)-1] + if first != last { + return value + } + if first == '\'' || first == '"' { + return value[1 : len(value)-1] + } + return value +} + +// splitEnvAssignment parses one KEY=VALUE, validating the name and value against +// the limits the server enforces. +func splitEnvAssignment(assignment string) (name, value string, err error) { + name, value, found := strings.Cut(assignment, "=") + if !found { + return "", "", fmt.Errorf("%q is not KEY%s", assignment, envAssignSuffix) + } + // The name is trimmed; the value is not, beyond the quotes below. Trailing + // whitespace in a value can be deliberate, and guessing costs more than it + // saves -- a token with a stray character is a 401 the app cannot explain. + name = strings.TrimSpace(name) + value = unquote(value) + + switch { + case name == "": + return "", "", fmt.Errorf("%q has an empty name", assignment) + // Counted in runes, not bytes: the API's maxLength is characters, so + // len() would reject a valid non-ASCII value at half the documented limit. + case utf8.RuneCountInString(name) > maxEnvNameLen: + return "", "", fmt.Errorf("environment variable name %q exceeds %d characters", name, maxEnvNameLen) + case !envNamePattern.MatchString(name): + return "", "", fmt.Errorf( + "environment variable name %q must be POSIX-style: letters, digits and underscore, not starting with a digit", + name, + ) + case utf8.RuneCountInString(value) > maxEnvValueLen: + return "", "", fmt.Errorf("value for %q exceeds %d characters", name, maxEnvValueLen) + } + return name, value, nil +} diff --git a/internal/cmd/serverless/env_test.go b/internal/cmd/serverless/env_test.go new file mode 100644 index 0000000..899a823 --- /dev/null +++ b/internal/cmd/serverless/env_test.go @@ -0,0 +1,243 @@ +package serverless + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBuildEnvironmentVariables(t *testing.T) { + got, err := buildEnvironmentVariables(nil, []string{"HF_TOKEN=abc", "LOG_LEVEL=debug"}) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if got == nil || len(*got) != 2 || (*got)["HF_TOKEN"] != "abc" { + t.Errorf("got %v", got) + } +} + +// Nothing supplied must send nil, not an empty map: the field is optional and an +// explicit {} is a different statement from saying nothing. +func TestBuildEnvironmentVariables_NoneIsNil(t *testing.T) { + got, err := buildEnvironmentVariables(nil, nil) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if got != nil { + t.Errorf("expected nil, got %v", *got) + } +} + +// A value may legitimately contain '=' -- base64 padding, a connection string -- +// so only the first separator splits. +func TestBuildEnvironmentVariables_ValueMayContainEquals(t *testing.T) { + got, err := buildEnvironmentVariables(nil, []string{"TOKEN=abc==def="}) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if (*got)["TOKEN"] != "abc==def=" { + t.Errorf("value = %q", (*got)["TOKEN"]) + } +} + +// An empty value is a legitimate assignment: unsetting by setting empty is how +// callers override a default the image bakes in. +func TestBuildEnvironmentVariables_EmptyValueAllowed(t *testing.T) { + got, err := buildEnvironmentVariables(nil, []string{"QUIET="}) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if v, ok := (*got)["QUIET"]; !ok || v != "" { + t.Errorf("got %v", *got) + } +} + +func TestBuildEnvironmentVariables_Rejects(t *testing.T) { + cases := []struct { + name string + pair string + want string + }{ + {name: "no separator", pair: "HF_TOKEN", want: "not KEY"}, + {name: "empty name", pair: "=value", want: "empty name"}, + {name: "leading digit", pair: "1BAD=x", want: "POSIX-style"}, + {name: "hyphen", pair: "BAD-NAME=x", want: "POSIX-style"}, + {name: "dot", pair: "bad.name=x", want: "POSIX-style"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := buildEnvironmentVariables(nil, []string{tc.pair}) + if err == nil { + t.Fatalf("expected an error for %q", tc.pair) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not mention %q", err, tc.want) + } + }) + } +} + +func TestBuildEnvironmentVariables_TooLongValue(t *testing.T) { + _, err := buildEnvironmentVariables(nil, []string{"BIG=" + strings.Repeat("x", maxEnvValueLen+1)}) + if err == nil || !strings.Contains(err.Error(), "exceeds") { + t.Fatalf("expected a length error, got %v", err) + } +} + +// The file form exists so a secret never lands in an argv, where it is visible +// in the process list and recorded in shell history. +func TestBuildEnvironmentVariables_FromFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".env.deploy") + content := "# a comment\n\nHF_TOKEN=from-file\nexport LOG_LEVEL=debug\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + + got, err := buildEnvironmentVariables([]string{path}, nil) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if (*got)["HF_TOKEN"] != "from-file" { + t.Errorf("HF_TOKEN = %q", (*got)["HF_TOKEN"]) + } + // `export FOO=bar` is what a shell-sourced file looks like; pasting one in + // should work rather than fail on the keyword. + if (*got)["LOG_LEVEL"] != "debug" { + t.Errorf("LOG_LEVEL = %q (export prefix not absorbed)", (*got)["LOG_LEVEL"]) + } + if len(*got) != 2 { + t.Errorf("comments or blank lines became entries: %v", *got) + } +} + +// An explicit --env is the more specific statement, so it wins over a file entry +// with the same name. +func TestBuildEnvironmentVariables_InlineOverridesFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".env.deploy") + if err := os.WriteFile(path, []byte("LOG_LEVEL=info\n"), 0o600); err != nil { + t.Fatal(err) + } + + got, err := buildEnvironmentVariables([]string{path}, []string{"LOG_LEVEL=debug"}) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if (*got)["LOG_LEVEL"] != "debug" { + t.Errorf("LOG_LEVEL = %q, want the inline value", (*got)["LOG_LEVEL"]) + } +} + +// A malformed line has to name the file and the line, or a long .env is a hunt. +func TestBuildEnvironmentVariables_FileErrorNamesTheLine(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".env.deploy") + if err := os.WriteFile(path, []byte("GOOD=1\nBROKEN\n"), 0o600); err != nil { + t.Fatal(err) + } + + _, err := buildEnvironmentVariables([]string{path}, nil) + if err == nil { + t.Fatal("expected an error for a malformed line") + } + if !strings.Contains(err.Error(), "line 2") { + t.Errorf("error %q does not name the line", err) + } +} + +func TestBuildEnvironmentVariables_MissingFile(t *testing.T) { + _, err := buildEnvironmentVariables([]string{filepath.Join(t.TempDir(), "nope")}, nil) + if err == nil { + t.Fatal("expected an error for a missing env file") + } +} + +func TestBuildEnvironmentVariables_TooMany(t *testing.T) { + pairs := make([]string, maxEnvVars+1) + for i := range pairs { + pairs[i] = "VAR_" + strings.Repeat("a", i%20) + string(rune('A'+i%26)) + string(rune('A'+i/26)) + "=x" + } + if _, err := buildEnvironmentVariables(nil, pairs); err == nil { + t.Fatal("expected an error past the variable limit") + } +} + +// A quoted value in an --env-file must arrive unquoted: shells strip quotes when +// sourcing, and passing them through sends `"hf_x"` as the token itself -- a 401 +// in the pod with nothing to read. +func TestBuildEnvironmentVariables_StripsSurroundingQuotes(t *testing.T) { + cases := []struct { + name string + line string + want string + }{ + {name: "double", line: `HF_TOKEN="hf_x"`, want: "hf_x"}, + {name: "single", line: `HF_TOKEN='hf_x'`, want: "hf_x"}, + // Only a matching pair, and only when it surrounds: a value that + // genuinely contains a quote keeps it. + {name: "unbalanced", line: `HF_TOKEN="hf_x`, want: `"hf_x`}, + {name: "mismatched", line: `HF_TOKEN='hf_x"`, want: `'hf_x"`}, + {name: "inner quote kept", line: `MSG=say "hi"`, want: `say "hi"`}, + {name: "one pair only", line: `MSG=""quoted""`, want: `"quoted"`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".env") + if err := os.WriteFile(path, []byte(tc.line+"\n"), 0o600); err != nil { + t.Fatal(err) + } + got, err := buildEnvironmentVariables([]string{path}, nil) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + for _, v := range *got { + if v != tc.want { + t.Errorf("value = %q, want %q", v, tc.want) + } + } + }) + } +} + +// Whitespace inside a value is the caller's business: trimming the whole line +// would silently rewrite a token or an indented value. +func TestBuildEnvironmentVariables_PreservesValueWhitespace(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, ".env") + // A trailing space in the value, and a leading-space line that still parses. + if err := os.WriteFile(path, []byte("PADDED=value \n INDENTED=x\n"), 0o600); err != nil { + t.Fatal(err) + } + + got, err := buildEnvironmentVariables([]string{path}, nil) + if err != nil { + t.Fatalf("buildEnvironmentVariables: %v", err) + } + if (*got)["PADDED"] != "value " { + t.Errorf("PADDED = %q, want %q", (*got)["PADDED"], "value ") + } + if (*got)["INDENTED"] != "x" { + t.Errorf("INDENTED = %q, want x", (*got)["INDENTED"]) + } +} + +// The API's limits are characters; len() counts bytes, so a byte check rejects a +// valid non-ASCII value at half the documented limit. +func TestBuildEnvironmentVariables_LimitIsCountedInRunes(t *testing.T) { + // 4096 two-byte runes: 4096 characters, 8192 bytes. + value := strings.Repeat("é", maxEnvValueLen) + got, err := buildEnvironmentVariables(nil, []string{"MSG=" + value}) + if err != nil { + t.Fatalf("a value of exactly the character limit was rejected: %v", err) + } + if (*got)["MSG"] != value { + t.Error("value did not survive") + } + // One rune past is still rejected. + if _, err := buildEnvironmentVariables(nil, []string{"MSG=" + value + "é"}); err == nil { + t.Error("expected a rejection one character past the limit") + } +} diff --git a/internal/cmd/serverless/pack.go b/internal/cmd/serverless/pack.go index 16e2e35..78802d1 100644 --- a/internal/cmd/serverless/pack.go +++ b/internal/cmd/serverless/pack.go @@ -6,68 +6,457 @@ import ( "encoding/base64" "fmt" "io" + "io/fs" "os" "path/filepath" + "sort" + "strings" + + "github.com/go-git/go-git/v5/plumbing/format/gitignore" ) -// maxPackEntryBytes is the maximum size of a single entry file we will zip and -// base64-encode for createApp. Keeps accidental large inputs from -// blowing memory (base64 expands the payload by ~4/3). +// maxPackEntryBytes is the maximum size of a single file we will put in the +// archive. Keeps one accidental artefact — a checkpoint, a dataset someone left +// in the project — from blowing memory, since the whole archive is held in +// memory and base64 expands it by ~4/3. const maxPackEntryBytes int64 = 10 << 20 // 10 MiB -// packPythonFile zips a single Python entry file and returns the base64-encoded -// archive plus the modelFile path expected inside the zip (the file's basename). -func packPythonFile(path string) (zipBase64, modelFile string, err error) { - info, err := os.Stat(path) +// maxPackTotalBytes bounds the archive as a whole. The per-file cap alone does +// not: a virtualenv is thousands of small files and would sail past it. +const maxPackTotalBytes int64 = 25 << 20 // 25 MiB + +// runwareIgnoreFile is the project's own exclude list, and the only one read. +// +// .gitignore is deliberately NOT consulted. What a project keeps out of version +// control is a different question from what it ships to a builder -- a generated +// asset the app needs at run time is a routine .gitignore entry -- and a file +// silently missing from a deployment because of a rule written for git is the +// kind of surprise that costs an afternoon. Exclusions are opt-in and local to +// this file. +const runwareIgnoreFile = ".runwareignore" + +// defaultIgnorePatterns are excluded when no rule says otherwise. They are +// ordinary gitignore patterns evaluated before the project's own, so a later +// `!*.pyc` re-includes what one of the file patterns here excluded. The +// directory patterns are not negatable the same way -- git does not allow +// re-including a file whose parent directory is excluded, and collectFiles +// prunes those directories for exactly that reason. +var defaultIgnorePatterns = []string{ + // Python build and cache output. None of it is source, and a virtualenv is + // the difference between an 80KB upload and a 400MB one. + "__pycache__/", + "*.pyc", + "*.pyo", + ".venv/", + "venv/", + "*.egg-info/", + // Tool caches, which appear in any project that has been tested or linted. + ".pytest_cache/", + ".ruff_cache/", + ".mypy_cache/", + ".tox/", + ".ipynb_checkpoints/", + // Everything else. + "node_modules/", + ".DS_Store", + runwareIgnoreFile, +} + +// alwaysExcluded reports paths that no rule may re-include. +// +// `.env` and its variants hold credentials, and the build unpacks this archive +// into a pod where its contents are readable — the builder keeps top-level +// dotfiles out of the *image*, but that is a later step and not a promise about +// the build. Matching on every path segment rather than the root alone, because +// `config/.env` is the same secret one directory down. `.git` goes with it: +// nothing in the build reads it, and it carries the whole history of everything +// the other rules exclude. +func alwaysExcluded(segments []string) bool { + for _, s := range segments { + if s == ".git" || s == ".env" || strings.HasPrefix(s, ".env.") { + return true + } + } + return false +} + +// packDirectory zips srcDir and returns the base64-encoded archive plus the path +// of modelFile inside it. +// +// srcDir is the archive root: every path in the zip is relative to it, which is +// what the builder puts on PYTHONPATH and hands to MLflow code_paths, so the +// project's own imports resolve at build and serve time exactly as they do +// locally. An empty srcDir means the working directory. +func packDirectory(srcDir, modelFile string) (zipBase64, modelFileRel string, err error) { + root, err := resolveSrcDir(srcDir) + if err != nil { + return "", "", err + } + + modelFileRel, err = relativeModelFile(root, modelFile) + if err != nil { + return "", "", err + } + + matcher, err := loadIgnoreMatcher(root) + if err != nil { + return "", "", err + } + + files, err := collectFiles(root, modelFileRel, matcher) + if err != nil { + return "", "", err + } + if len(files) == 0 { + // Unreachable while the model file is forced in, but a packer that can + // return an empty archive should say so rather than let the builder + // answer with a 422 about a missing model file. + return "", "", fmt.Errorf("no files to pack under %q", root) + } + + raw, err := writeArchive(root, files) + if err != nil { + return "", "", err + } + return base64.StdEncoding.EncodeToString(raw), modelFileRel, nil +} + +// resolveSrcDir defaults an empty srcDir to the working directory and checks it +// is a directory. Symlinks are resolved so the relative path of the model file +// is computed against the same tree WalkDir will produce. +func resolveSrcDir(srcDir string) (string, error) { + if srcDir == "" { + wd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("resolve working directory: %w", err) + } + srcDir = wd + } + abs, err := filepath.Abs(srcDir) + if err != nil { + return "", fmt.Errorf("resolve source directory: %w", err) + } + resolved, err := filepath.EvalSymlinks(abs) + if err != nil { + return "", fmt.Errorf("read source directory %q: %w", srcDir, err) + } + info, err := os.Stat(resolved) + if err != nil { + return "", fmt.Errorf("read source directory %q: %w", srcDir, err) + } + if !info.IsDir() { + return "", fmt.Errorf("source directory %q is not a directory", srcDir) + } + return resolved, nil +} + +// relativeModelFile locates the model file inside the archive root. +// +// The builder rejects a modelFile that is absolute or escapes the codebase, and +// answers 422 for a modelFile it cannot find in the zip. Both are worth catching +// here: the failure is the caller's to fix and the archive may be megabytes. +func relativeModelFile(root, modelFile string) (string, error) { + if modelFile == "" { + return "", fmt.Errorf("model file is required") + } + abs := locateModelFile(root, modelFile) + info, err := os.Stat(abs) if err != nil { - return "", "", fmt.Errorf("read entry file: %w", err) + return "", fmt.Errorf( + "read model file %q: %w (relative paths are resolved inside the source directory %s)", + modelFile, err, root, + ) } if info.IsDir() { - return "", "", fmt.Errorf("entry file %q is a directory", path) - } - if info.Size() > maxPackEntryBytes { - return "", "", fmt.Errorf( - "entry file %q is %d bytes; maximum supported size is %d bytes (%d MiB)", - path, - info.Size(), - maxPackEntryBytes, - maxPackEntryBytes>>20, + return "", fmt.Errorf("model file %q is a directory", modelFile) + } + // EvalSymlinks on the parent only: a model file reached through a symlinked + // directory still belongs to the tree, and resolving the file itself would + // reject the common case of an editor's saved-through link. + parent, err := filepath.EvalSymlinks(filepath.Dir(abs)) + if err != nil { + return "", fmt.Errorf("read model file %q: %w", modelFile, err) + } + abs = filepath.Join(parent, filepath.Base(abs)) + + rel, err := filepath.Rel(root, abs) + if err != nil { + return "", fmt.Errorf("locate model file inside %q: %w", root, err) + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf( + "model file %q is outside the source directory %q; pass --src-dir pointing at the project root", + modelFile, root, ) } + return filepath.ToSlash(rel), nil +} + +// locateModelFile resolves a model file argument to an absolute path. +// +// One rule: absolute is taken as given, relative is relative to the source +// directory. modelFile is a path *inside* the codebase -- that is what the field +// means on the wire and what the builder looks up in the zip -- so the source +// directory is the only sensible thing for it to be relative to. With no +// --src-dir the source directory is the working directory, so `deploy ./app.py` +// from the project means what it always did. +func locateModelFile(root, modelFile string) string { + if filepath.IsAbs(modelFile) { + return modelFile + } + return filepath.Join(root, modelFile) +} + +// loadIgnoreMatcher builds the exclusion matcher: the built-in defaults first, +// then the project's own .runwareignore, so a project rule can override a +// default. Nothing else is read -- see runwareIgnoreFile for why .gitignore is +// not. +func loadIgnoreMatcher(root string) (gitignore.Matcher, error) { + patterns := make([]gitignore.Pattern, 0, len(defaultIgnorePatterns)) + for _, p := range defaultIgnorePatterns { + patterns = append(patterns, gitignore.ParsePattern(p, nil)) + } + + lines, err := readIgnoreFile(filepath.Join(root, runwareIgnoreFile)) + if err != nil { + return nil, err + } + for _, line := range lines { + patterns = append(patterns, gitignore.ParsePattern(line, nil)) + } + + return gitignore.NewMatcher(patterns), nil +} + +// readIgnoreFile returns the file's lines, or nil when it is absent. +func readIgnoreFile(path string) ([]string, error) { + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("read %s: %w", filepath.Base(path), err) + } - modelFile = filepath.Base(path) - if modelFile == "." || modelFile == string(filepath.Separator) { - return "", "", fmt.Errorf("invalid entry file path %q", path) + // Lines are passed through verbatim apart from a CR: gitignore syntax is + // whitespace-significant, so trimming would rewrite valid patterns -- an + // escaped trailing space (`name\ `) loses its escape and a leading space is + // part of the pattern. Blank lines and comments are the parser's own + // business; ParsePattern handles both, and a pattern that begins with an + // escaped `#` must not be mistaken for one here. + lines := []string{} + for _, line := range strings.Split(string(raw), "\n") { + lines = append(lines, strings.TrimSuffix(line, "\r")) } + return lines, nil +} - f, err := os.Open(path) +// packedFile is one archive entry: its path relative to the root, and its size. +type packedFile struct { + rel string + size int64 +} + +// collectFiles walks the tree and returns what belongs in the archive, in the +// lexical order WalkDir yields — so the same tree always packs to the same bytes. +func collectFiles(root, modelFileRel string, matcher gitignore.Matcher) ([]packedFile, error) { + var files []packedFile + var total int64 + + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if path == root { + return nil + } + + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + rel = filepath.ToSlash(rel) + segments := strings.Split(rel, "/") + + if alwaysExcluded(segments) { + if d.IsDir() { + return fs.SkipDir + } + return nil + } + // The model file is packed whatever the rules say: the deploy cannot + // succeed without it, and an ignore rule that happens to cover it is a + // worse failure than a file the customer did not mean to ship. + if rel != modelFileRel && matcher.Match(segments, d.IsDir()) { + if d.IsDir() { + // An excluded directory is pruned rather than walked, which keeps + // a .venv from costing a stat per file and reproduces git's own + // rule that a negation cannot re-include a file whose parent + // directory is excluded. + // + // Except when the model file is inside it. Pruning there would + // drop the one entry the build cannot proceed without, and the + // exemption above never fires because the walk stops at the + // directory, whose path is not the model file's. Descend, and let + // the per-file checks exclude everything else it holds. + if isAncestorOf(rel, modelFileRel) { + return nil + } + return fs.SkipDir + } + return nil + } + if d.IsDir() { + return nil + } + // Regular files only. A symlink may point outside the tree, and the + // archive is a copy rather than a checkout; sockets and devices have + // nothing to copy. + if !d.Type().IsRegular() { + // Silently skipping the model file would upload an archive whose + // declared entry point is missing, which the builder can only report + // as a 422 after the upload. Say it here instead. + if rel == modelFileRel { + return fmt.Errorf( + "model file %s is a %s, not a regular file; point --src-dir at the directory holding the real file", + rel, d.Type().String(), + ) + } + return nil + } + + info, err := d.Info() + if err != nil { + return err + } + if info.Size() > maxPackEntryBytes { + return fmt.Errorf( + "%s is %s; the maximum for a single file is %s. Exclude it with a %s rule", + rel, humanBytes(info.Size()), humanBytes(maxPackEntryBytes), runwareIgnoreFile, + ) + } + total += info.Size() + files = append(files, packedFile{rel: rel, size: info.Size()}) + return nil + }) if err != nil { - return "", "", fmt.Errorf("open entry file: %w", err) + return nil, err + } + + if total > maxPackTotalBytes { + return nil, fmt.Errorf( + "the source directory holds %s of files to pack; the maximum is %s.\n%s\nExclude what the app does not need with a %s file", + humanBytes(total), humanBytes(maxPackTotalBytes), largestFilesSummary(files), runwareIgnoreFile, + ) + } + return files, nil +} + +// isAncestorOf reports whether dir is a parent directory of file, both being +// slash-separated paths relative to the archive root. +func isAncestorOf(dir, file string) bool { + return strings.HasPrefix(file, dir+"/") +} + +// largestFilesSummary names what filled the archive. "Too big" on its own leaves +// the caller to find the offender by hand, which for a deep tree is the whole +// problem rather than a detail of it. +func largestFilesSummary(files []packedFile) string { + sorted := make([]packedFile, len(files)) + copy(sorted, files) + sort.Slice(sorted, func(i, j int) bool { return sorted[i].size > sorted[j].size }) + + var b strings.Builder + b.WriteString("Largest files:") + for i, f := range sorted { + if i == 5 { + break + } + fmt.Fprintf(&b, "\n %8s %s", humanBytes(f.size), f.rel) + } + return b.String() +} + +func humanBytes(n int64) string { + switch { + case n >= 1<<20: + return fmt.Sprintf("%.1f MiB", float64(n)/(1<<20)) + case n >= 1<<10: + return fmt.Sprintf("%.1f KiB", float64(n)/(1<<10)) + default: + return fmt.Sprintf("%d B", n) } - defer f.Close() //nolint:errcheck +} +// writeArchive builds the zip. Entry names are the already-slashed relative +// paths: zip names are always "/"-separated, which is what makes an archive +// packed on Windows unpack correctly in the Linux build pod. +func writeArchive(root string, files []packedFile) ([]byte, error) { var buf bytes.Buffer zw := zip.NewWriter(&buf) - w, err := zw.Create(modelFile) + + // Counted here rather than trusted from the walk: a file may grow between + // being stat'd and being read, and several files each staying under the + // per-file cap can still cross the total. + var written int64 + for _, f := range files { + n, err := writeArchiveEntry(zw, root, f) + if err != nil { + return nil, err + } + written += n + if written > maxPackTotalBytes { + return nil, fmt.Errorf( + "the files grew past the %s archive limit while packing; exclude what the app does not need with a %s file", + humanBytes(maxPackTotalBytes), runwareIgnoreFile, + ) + } + } + if err := zw.Close(); err != nil { + return nil, fmt.Errorf("close zip: %w", err) + } + return buf.Bytes(), nil +} + +func writeArchiveEntry(zw *zip.Writer, root string, f packedFile) (int64, error) { + path := filepath.Join(root, filepath.FromSlash(f.rel)) + src, err := os.Open(path) + if err != nil { + return 0, fmt.Errorf("open %s: %w", f.rel, err) + } + defer src.Close() //nolint:errcheck + + info, err := src.Stat() + if err != nil { + return 0, fmt.Errorf("stat %s: %w", f.rel, err) + } + // FileInfoHeader rather than zw.Create, because Create writes mode 0. A + // codebase can hold an executable -- an entrypoint script, a helper binary -- + // and one that arrives without its executable bit fails at run time with + // nothing about the archive to explain it. The name has to be re-set: the + // header takes it from the FileInfo, which knows only the base name. + header, err := zip.FileInfoHeader(info) if err != nil { - return "", "", fmt.Errorf("create zip entry: %w", err) + return 0, fmt.Errorf("build zip header for %s: %w", f.rel, err) } - // Cap the copy in case the file grows after Stat. - written, err := io.Copy(w, io.LimitReader(f, maxPackEntryBytes+1)) + header.Name = f.rel + header.Method = zip.Deflate + + w, err := zw.CreateHeader(header) + if err != nil { + return 0, fmt.Errorf("create zip entry %s: %w", f.rel, err) + } + // Capped in case the file grew between the walk and here, so a file that + // changes underneath us cannot defeat the per-file limit checked above. + written, err := io.Copy(w, io.LimitReader(src, maxPackEntryBytes+1)) if err != nil { - return "", "", fmt.Errorf("write zip entry: %w", err) + return 0, fmt.Errorf("write zip entry %s: %w", f.rel, err) } if written > maxPackEntryBytes { - return "", "", fmt.Errorf( - "entry file %q exceeds maximum supported size of %d bytes (%d MiB)", - path, - maxPackEntryBytes, - maxPackEntryBytes>>20, + return 0, fmt.Errorf( + "%s grew past the maximum for a single file (%s) while packing", + f.rel, humanBytes(maxPackEntryBytes), ) } - if err := zw.Close(); err != nil { - return "", "", fmt.Errorf("close zip: %w", err) - } - - return base64.StdEncoding.EncodeToString(buf.Bytes()), modelFile, nil + return written, nil } diff --git a/internal/cmd/serverless/pack_test.go b/internal/cmd/serverless/pack_test.go index 6201b46..b820718 100644 --- a/internal/cmd/serverless/pack_test.go +++ b/internal/cmd/serverless/pack_test.go @@ -4,75 +4,489 @@ import ( "archive/zip" "bytes" "encoding/base64" + "io" "os" "path/filepath" + "runtime" + "sort" "strings" "testing" ) -func TestPackPythonFile(t *testing.T) { +// writeTree materialises files under dir. Keys are slash-separated relative +// paths; parent directories are created as needed. +func writeTree(t *testing.T, dir string, files map[string]string) { + t.Helper() + for rel, content := range files { + path := filepath.Join(dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } +} + +// unpack decodes an archive into path -> contents. +func unpack(t *testing.T, encoded string) map[string]string { + t.Helper() + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatalf("decode: %v", err) + } + zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw))) + if err != nil { + t.Fatalf("zip.NewReader: %v", err) + } + + out := make(map[string]string, len(zr.File)) + for _, f := range zr.File { + rc, err := f.Open() + if err != nil { + t.Fatal(err) + } + content, err := io.ReadAll(rc) + _ = rc.Close() + if err != nil { + t.Fatal(err) + } + out[f.Name] = string(content) + } + return out +} + +// Repeated across the table-free tests below; goconst wants them named. +const ( + testModelFile = "app.py" + testPySource = "x = 1\n" +) + +func names(packed map[string]string) []string { + out := make([]string, 0, len(packed)) + for name := range packed { + out = append(out, name) + } + sort.Strings(out) + return out +} + +// TestPackDirectory proves the whole project ships, not just the entry file: +// nested modules and data files keep the relative paths the app imports them by, +// which is what the builder puts on PYTHONPATH. +func TestPackDirectory(t *testing.T) { dir := t.TempDir() - path := filepath.Join(dir, "app.py") - content := []byte("def predict():\n return 1\n") - if err := os.WriteFile(path, content, 0o600); err != nil { - t.Fatal(err) + writeTree(t, dir, map[string]string{ + testModelFile: "import lib\n", + "lib/__init__.py": "", + "lib/helpers.py": "def go(): pass\n", + "data/prompt.txt": "hello", + "nested/deep/x.py": testPySource, + }) + + encoded, modelFile, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if modelFile != testModelFile { + t.Errorf("modelFile = %q, want app.py", modelFile) } - encoded, modelFile, err := packPythonFile(path) + packed := unpack(t, encoded) + for _, want := range []string{testModelFile, "lib/helpers.py", "data/prompt.txt", "nested/deep/x.py"} { + if _, ok := packed[want]; !ok { + t.Errorf("%q missing from the archive; got %v", want, names(packed)) + } + } + if packed["lib/helpers.py"] != "def go(): pass\n" { + t.Errorf("lib/helpers.py content = %q", packed["lib/helpers.py"]) + } +} + +// TestPackDirectory_DefaultsToWorkingDirectory covers `deploy ./app.py` with no +// --src-dir, which is the common invocation. +func TestPackDirectory_DefaultsToWorkingDirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: testPySource, + "lib.py": "y = 2\n", + }) + t.Chdir(dir) + + encoded, modelFile, err := packDirectory("", testModelFile) if err != nil { - t.Fatalf("packPythonFile: %v", err) + t.Fatalf("packDirectory: %v", err) } - if modelFile != "app.py" { + if modelFile != testModelFile { t.Errorf("modelFile = %q, want app.py", modelFile) } + if got := names(unpack(t, encoded)); len(got) != 2 { + t.Errorf("archive = %v, want app.py and lib.py", got) + } +} - raw, err := base64.StdEncoding.DecodeString(encoded) +// TestPackDirectory_ModelFileInSubdirectory proves modelFile is reported as the +// path inside the archive, which is what the builder looks up. +func TestPackDirectory_ModelFileInSubdirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{"src/app.py": testPySource}) + + _, modelFile, err := packDirectory(dir, filepath.Join(dir, "src", testModelFile)) if err != nil { - t.Fatalf("decode: %v", err) + t.Fatalf("packDirectory: %v", err) } - zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw))) + if modelFile != "src/app.py" { + t.Errorf("modelFile = %q, want src/app.py", modelFile) + } +} + +// TestPackDirectory_ModelFileOutsideRoot fails locally rather than uploading an +// archive the builder will reject with a 422. +func TestPackDirectory_ModelFileOutsideRoot(t *testing.T) { + base := t.TempDir() + root := filepath.Join(base, "project") + writeTree(t, root, map[string]string{"keep.py": ""}) + writeTree(t, base, map[string]string{"outside.py": ""}) + + _, _, err := packDirectory(root, filepath.Join(base, "outside.py")) + if err == nil { + t.Fatal("expected an error for a model file outside the source directory") + } + if !strings.Contains(err.Error(), "--src-dir") { + t.Errorf("error %q does not say how to fix it", err) + } +} + +// TestPackDirectory_DefaultExcludes proves the built-in list keeps the usual +// noise out without any ignore file present. +func TestPackDirectory_DefaultExcludes(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + "__pycache__/app.pyc": "", + "lib/__pycache__/mod.pyc": "", + "lib/mod.py": "", + ".venv/lib/python/x.py": "", + "node_modules/pkg/i.js": "", + ".git/config": "", + ".DS_Store": "", + }) + + encoded, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) if err != nil { - t.Fatalf("zip.NewReader: %v", err) + t.Fatalf("packDirectory: %v", err) } - if len(zr.File) != 1 { - t.Fatalf("expected 1 zip entry, got %d", len(zr.File)) + + got := names(unpack(t, encoded)) + want := []string{testModelFile, "lib/mod.py"} + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Errorf("archive = %v, want %v", got, want) } - if zr.File[0].Name != "app.py" { - t.Errorf("zip entry name = %q, want app.py", zr.File[0].Name) +} + +// TestPackDirectory_RunwareIgnore proves the project's own rules apply, that a +// negation re-includes a file an earlier rule excluded, and that comments and +// blank lines are ignored. +func TestPackDirectory_RunwareIgnore(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + "outputs/result.png": "", + "notes.md": "", + "README.md": "", + "cached/x.pyc": "", + runwareIgnoreFile: "outputs/\n*.md\n\n# a comment\n!README.md\n", + }) + + encoded, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) } - rc, err := zr.File[0].Open() + packed := unpack(t, encoded) + for _, absent := range []string{"outputs/result.png", "notes.md", runwareIgnoreFile, "cached/x.pyc"} { + if _, ok := packed[absent]; ok { + t.Errorf("%q should have been excluded; archive = %v", absent, names(packed)) + } + } + // A later ! rule wins over the earlier *.md that excluded it. + if _, ok := packed["README.md"]; !ok { + t.Errorf("a ! rule did not re-include README.md; archive = %v", names(packed)) + } +} + +// TestPackDirectory_NegationCannotReachIntoExcludedDirectory pins git's own rule: +// "It is not possible to re-include a file if a parent directory of that file is +// excluded." The packer prunes an excluded directory rather than walking it, so a +// negation inside one never applies -- the same result git gives, and the reason +// a .venv costs nothing to skip. +func TestPackDirectory_NegationCannotReachIntoExcludedDirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + "node_modules/keep.js": "", + runwareIgnoreFile: "!node_modules/keep.js\n", + }) + + encoded, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if _, ok := unpack(t, encoded)["node_modules/keep.js"]; ok { + t.Error("a negation reached inside an excluded directory; git does not allow that") + } +} + +// TestPackDirectory_GitignoreIsNotRead pins the decision that .gitignore has no +// effect. What a project keeps out of version control is a different question +// from what it ships: a generated asset the app needs at run time is a routine +// .gitignore entry, and a file silently missing from a deployment because of a +// rule written for git is a surprise nobody can debug from the outside. +func TestPackDirectory_GitignoreIsNotRead(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + "secret.txt": "", + "build/o.so": "", + ".gitignore": "secret.txt\nbuild/\n", + }) + + encoded, _, err := packDirectory(dir, testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + packed := unpack(t, encoded) + for _, present := range []string{"secret.txt", "build/o.so"} { + if _, ok := packed[present]; !ok { + t.Errorf(".gitignore excluded %q; only .runwareignore may exclude", present) + } + } + // The file itself ships like any other: it is not special, just not obeyed. + if _, ok := packed[".gitignore"]; !ok { + t.Errorf(".gitignore should ship as an ordinary file; archive = %v", names(packed)) + } +} + +// TestPackDirectory_NeverPacksEnvFiles is the rule that must not be overridable: +// .env files hold credentials and the build pod unpacks whatever it is sent, so +// no ignore rule — not even an explicit negation — may re-include one. +func TestPackDirectory_NeverPacksEnvFiles(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + ".env": "SECRET=1", + ".env.local": "SECRET=2", + ".env.production": "SECRET=3", + "config/.env": "SECRET=4", + "deep/nest/.env.ci": "SECRET=5", + "envoy.py": "not an env file", + ".runwareignore": "!.env\n!.env.*\n!config/.env\n", + }) + + encoded, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + + packed := unpack(t, encoded) + for name, content := range packed { + if strings.Contains(content, "SECRET=") { + t.Errorf("%q carries an env secret into the archive", name) + } + } + for _, absent := range []string{".env", ".env.local", ".env.production", "config/.env", "deep/nest/.env.ci"} { + if _, ok := packed[absent]; ok { + t.Errorf("%q was packed despite the absolute exclusion; archive = %v", absent, names(packed)) + } + } + // The rule matches env files, not every name that starts with "env". + if _, ok := packed["envoy.py"]; !ok { + t.Errorf("envoy.py was wrongly treated as an env file; archive = %v", names(packed)) + } +} + +// TestPackDirectory_NeverPacksGitDirectory is the second half of the absolute +// exclusions: .git holds the whole history of everything the other rules exclude +// -- including any .env ever committed -- so no rule may re-include it, and a +// nested repo one directory down is the same problem. +func TestPackDirectory_NeverPacksGitDirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + ".git/config": "[core]", + ".git/objects/ab/cdef": "history", + "vendor/dep/.git/config": "[core]", + "gitignore-notes.md": "not a git directory", + runwareIgnoreFile: "!.git\n!.git/**\n!vendor/dep/.git/**\n", + }) + + encoded, _, err := packDirectory(dir, testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + + packed := unpack(t, encoded) + for name := range packed { + if strings.Contains(name, ".git/") { + t.Errorf("%q was packed despite the absolute exclusion; archive = %v", name, names(packed)) + } + } + // The rule matches the directory, not every name containing "git". + if _, ok := packed["gitignore-notes.md"]; !ok { + t.Errorf("gitignore-notes.md was wrongly treated as a git directory; archive = %v", names(packed)) + } +} + +// TestPackDirectory_ModelFileAlwaysPacked proves an ignore rule covering the +// entry file cannot produce an archive the build is guaranteed to reject. +func TestPackDirectory_ModelFileAlwaysPacked(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "x = 1\n", + runwareIgnoreFile: "*.py\n", + }) + + encoded, modelFile, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + packed := unpack(t, encoded) + if _, ok := packed[modelFile]; !ok { + t.Errorf("the model file was excluded by an ignore rule; archive = %v", names(packed)) + } +} + +// TestPackDirectory_SkipsSymlinks: a symlink may point outside the tree, and the +// archive is a copy rather than a checkout. +func TestPackDirectory_SkipsSymlinks(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation needs privileges on Windows") + } + dir := t.TempDir() + writeTree(t, dir, map[string]string{testModelFile: ""}) + outside := filepath.Join(t.TempDir(), "outside.txt") + if err := os.WriteFile(outside, []byte("elsewhere"), 0o600); err != nil { t.Fatal(err) } - defer rc.Close() //nolint:errcheck - got := new(bytes.Buffer) - if _, err := got.ReadFrom(rc); err != nil { + if err := os.Symlink(outside, filepath.Join(dir, "link.txt")); err != nil { t.Fatal(err) } - if !bytes.Equal(got.Bytes(), content) { - t.Errorf("zip contents = %q, want %q", got.Bytes(), content) + + encoded, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if _, ok := unpack(t, encoded)["link.txt"]; ok { + t.Error("a symlink was followed into the archive") } } -func TestPackPythonFile_Missing(t *testing.T) { - _, _, err := packPythonFile(filepath.Join(t.TempDir(), "missing.py")) +// TestPackDirectory_Deterministic: the same tree must pack to the same bytes, so +// a redeploy of unchanged source is visibly unchanged. +func TestPackDirectory_Deterministic(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: "", + "b/two.py": "", + "a/one.py": "", + }) + + first, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + second, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if first != second { + t.Error("packing the same tree twice produced different archives") + } +} + +// TestPackDirectory_ModelFileRelativeToSrcDir is the invocation the --src-dir +// flag exists for: name the entry file as it sits inside the project, from +// wherever you happen to be standing. +func TestPackDirectory_ModelFileRelativeToSrcDir(t *testing.T) { + project := t.TempDir() + writeTree(t, project, map[string]string{testModelFile: testPySource}) + t.Chdir(t.TempDir()) // stand somewhere else entirely + + _, modelFile, err := packDirectory(project, testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if modelFile != testModelFile { + t.Errorf("modelFile = %q, want %q", modelFile, testModelFile) + } +} + +// TestPackDirectory_ModelFileAbsolute: an absolute path is taken as given, which +// is the other half of the rule and what shell completion produces when the +// project is somewhere else. +func TestPackDirectory_ModelFileAbsolute(t *testing.T) { + project := t.TempDir() + writeTree(t, project, map[string]string{"src/" + testModelFile: testPySource}) + t.Chdir(t.TempDir()) + + _, modelFile, err := packDirectory(project, filepath.Join(project, "src", testModelFile)) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if modelFile != "src/"+testModelFile { + t.Errorf("modelFile = %q, want src/%s", modelFile, testModelFile) + } +} + +// TestPackDirectory_RelativeModelFileIsNotWorkingDirRelative pins the rule that +// makes the two above unambiguous: a relative path is resolved inside the source +// directory and nowhere else, so a file that exists in the working directory but +// not in the project is an error rather than a silently different app. +func TestPackDirectory_RelativeModelFileIsNotWorkingDirRelative(t *testing.T) { + project := t.TempDir() + writeTree(t, project, map[string]string{"real.py": testPySource}) + + elsewhere := t.TempDir() + writeTree(t, elsewhere, map[string]string{testModelFile: "the wrong file\n"}) + t.Chdir(elsewhere) + + _, _, err := packDirectory(project, testModelFile) + if err == nil { + t.Fatal("expected an error: the model file exists in the working directory, not the source directory") + } + if !strings.Contains(err.Error(), "source directory") { + t.Errorf("error %q does not explain where relative paths resolve", err) + } +} + +func TestPackDirectory_MissingModelFile(t *testing.T) { + dir := t.TempDir() + _, _, err := packDirectory(dir, filepath.Join(dir, "missing.py")) + if err == nil { + t.Fatal("expected an error for a missing model file") + } +} + +func TestPackDirectory_MissingSourceDirectory(t *testing.T) { + _, _, err := packDirectory(filepath.Join(t.TempDir(), "nope"), testModelFile) if err == nil { - t.Fatal("expected error for missing file") + t.Fatal("expected an error for a missing source directory") } } -func TestPackPythonFile_Directory(t *testing.T) { - _, _, err := packPythonFile(t.TempDir()) +func TestPackDirectory_ModelFileIsDirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{"pkg/app.py": ""}) + _, _, err := packDirectory(dir, filepath.Join(dir, "pkg")) if err == nil { - t.Fatal("expected error for directory") + t.Fatal("expected an error when the model file is a directory") } } -func TestPackPythonFile_TooLarge(t *testing.T) { +func TestPackDirectory_FileTooLarge(t *testing.T) { dir := t.TempDir() - path := filepath.Join(dir, "huge.py") - f, err := os.Create(path) + writeTree(t, dir, map[string]string{testModelFile: ""}) + f, err := os.Create(filepath.Join(dir, "huge.bin")) if err != nil { t.Fatal(err) } @@ -84,11 +498,200 @@ func TestPackPythonFile_TooLarge(t *testing.T) { t.Fatal(err) } - _, _, err = packPythonFile(path) + _, _, err = packDirectory(dir, filepath.Join(dir, testModelFile)) + if err == nil { + t.Fatal("expected an error for an oversized file") + } + // The message has to name the file and the remedy, or the caller is left + // hunting through a deep tree for whichever file it meant. + if !strings.Contains(err.Error(), "huge.bin") || !strings.Contains(err.Error(), runwareIgnoreFile) { + t.Errorf("error %q does not name the file and the remedy", err) + } +} + +func TestPackDirectory_TotalTooLarge(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{testModelFile: ""}) + // Several files, each under the per-file cap, over the total. + for _, name := range []string{"a.bin", "b.bin", "c.bin"} { + f, err := os.Create(filepath.Join(dir, name)) + if err != nil { + t.Fatal(err) + } + if err := f.Truncate(maxPackEntryBytes); err != nil { + _ = f.Close() + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + } + + _, _, err := packDirectory(dir, filepath.Join(dir, testModelFile)) + if err == nil { + t.Fatal("expected an error for an oversized archive") + } + if !strings.Contains(err.Error(), "Largest files") { + t.Errorf("error %q does not say what filled the archive", err) + } +} + +// TestPackDirectory_ModelFileAlwaysPackedFromExcludedDirectory is the reviewer's +// case: the exemption for the model file has to survive an ignore rule that +// matches one of its ANCESTORS. The walk meets the directory first, and the +// directory's path is not the model file's, so a naive prune drops the one entry +// the build cannot proceed without. +func TestPackDirectory_ModelFileAlwaysPackedFromExcludedDirectory(t *testing.T) { + const nestedModelFile = "dist/" + testModelFile + + cases := []struct { + name string + files map[string]string + }{ + { + name: "runwareignore directory rule", + files: map[string]string{ + runwareIgnoreFile: "dist/\n", + nestedModelFile: testPySource, + }, + }, + { + name: "built-in default directory rule", + files: map[string]string{ + "node_modules/" + testModelFile: testPySource, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, tc.files) + // The model file for the built-in case sits under node_modules. + model := nestedModelFile + if _, ok := tc.files[nestedModelFile]; !ok { + model = "node_modules/" + testModelFile + } + + encoded, modelFile, err := packDirectory(dir, model) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + if modelFile != model { + t.Errorf("modelFile = %q, want %q", modelFile, model) + } + if _, ok := unpack(t, encoded)[model]; !ok { + t.Errorf("the model file was pruned with its directory; archive = %v", names(unpack(t, encoded))) + } + }) + } +} + +// Everything else in an excluded directory stays excluded -- descending for the +// model file must not turn the prune into a free pass for its siblings. +func TestPackDirectory_ExcludedDirectoryKeepsExcludingSiblings(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + runwareIgnoreFile: "dist/\n", + "dist/" + testModelFile: testPySource, + "dist/junk.bin": "junk", + "dist/deep/more.bin": "junk", + }) + + encoded, _, err := packDirectory(dir, "dist/"+testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + packed := unpack(t, encoded) + for _, absent := range []string{"dist/junk.bin", "dist/deep/more.bin"} { + if _, ok := packed[absent]; ok { + t.Errorf("%q rode along with the model file; archive = %v", absent, names(packed)) + } + } +} + +// The executable bit has to survive the archive: a codebase can hold an +// entrypoint script, and one that arrives non-executable fails at run time with +// nothing about the archive to explain it. +func TestPackDirectory_PreservesFileMode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no executable bit to preserve on Windows") + } + dir := t.TempDir() + writeTree(t, dir, map[string]string{testModelFile: testPySource, "entrypoint.sh": "#!/bin/sh\n"}) + // 0o755 is the point of the test: the executable bit is what has to survive + // the archive, so gosec's 0600 ceiling cannot apply here. + if err := os.Chmod(filepath.Join(dir, "entrypoint.sh"), 0o755); err != nil { //nolint:gosec + t.Fatal(err) + } + + encoded, _, err := packDirectory(dir, testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + raw, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatal(err) + } + zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw))) + if err != nil { + t.Fatal(err) + } + for _, f := range zr.File { + if f.Name != "entrypoint.sh" { + continue + } + if f.Mode().Perm()&0o111 == 0 { + t.Errorf("entrypoint.sh lost its executable bit: mode %v", f.Mode()) + } + return + } + t.Fatal("entrypoint.sh missing from the archive") +} + +// A symlink as the model file used to pass the local stat and then be skipped by +// the walk, uploading an archive whose declared entry point is absent. +func TestPackDirectory_SymlinkModelFileRejected(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation needs privileges on Windows") + } + dir := t.TempDir() + writeTree(t, dir, map[string]string{"real.py": testPySource}) + if err := os.Symlink(filepath.Join(dir, "real.py"), filepath.Join(dir, "link.py")); err != nil { + t.Fatal(err) + } + + _, _, err := packDirectory(dir, "link.py") if err == nil { - t.Fatal("expected error for oversized entry file") + t.Fatal("expected an error for a symlinked model file") + } + if !strings.Contains(err.Error(), "regular file") { + t.Errorf("error %q does not explain the problem", err) + } +} + +// Ignore patterns are whitespace-significant, so the file's lines must reach the +// parser unmodified: a trailing space is escapable and a leading space is part of +// the pattern. +func TestPackDirectory_IgnorePatternsAreNotTrimmed(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + testModelFile: testPySource, + "keep me.txt": "kept", + "drop.txt": "dropped", + runwareIgnoreFile: "drop.txt\r\n", + }) + + encoded, _, err := packDirectory(dir, testModelFile) + if err != nil { + t.Fatalf("packDirectory: %v", err) + } + packed := unpack(t, encoded) + // A CRLF file must still work: only the CR is normalised. + if _, ok := packed["drop.txt"]; ok { + t.Errorf("a CRLF ignore line was not honoured; archive = %v", names(packed)) } - if !strings.Contains(err.Error(), "maximum supported size") { - t.Errorf("error %q does not mention size limit", err) + if _, ok := packed["keep me.txt"]; !ok { + t.Errorf("an unrelated file with a space was dropped; archive = %v", names(packed)) } } diff --git a/internal/cmd/serverless/volume.go b/internal/cmd/serverless/volume.go new file mode 100644 index 0000000..c6c9034 --- /dev/null +++ b/internal/cmd/serverless/volume.go @@ -0,0 +1,94 @@ +package serverless + +import ( + "fmt" + "path" + "strings" + + serverlessapi "github.com/runware/runware-cli/internal/api/serverless" +) + +// Volume limits, mirrored from the server. A local copy is worth its upkeep: a +// code deployment can build for up to ninety minutes, and learning only then +// that two mounts overlap is the whole of that wait wasted. The server validates +// every request independently regardless -- this is a courtesy, not a boundary. +const ( + maxVolumes = 30 + maxVolumeMountPathLen = 2048 + maxVolumePathComponent = 255 + // The characters the server's mountPath pattern allows besides letters and + // digits: ^/[A-Za-z0-9._/+@-]+$ + volumeMountPathExtra = "._-/+@" +) + +// buildVolumes turns --volume flags into the wire type, rejecting anything the +// server would reject. +// +// The mount path is the volume's whole identity: there is no name, because the +// path is what the app opens and what the node-local directory is keyed by. So +// nothing here needs a second field, and two entries that resolve to the same +// place are a mistake rather than a merge. +func buildVolumes(mountPaths []string) (*[]serverlessapi.AppVolume, error) { + if len(mountPaths) == 0 { + return nil, nil + } + if len(mountPaths) > maxVolumes { + return nil, fmt.Errorf("at most %d volumes (got %d)", maxVolumes, len(mountPaths)) + } + + volumes := make([]serverlessapi.AppVolume, 0, len(mountPaths)) + seen := make([]string, 0, len(mountPaths)) + for _, raw := range mountPaths { + mount, err := validateMountPath(raw, seen) + if err != nil { + return nil, err + } + seen = append(seen, mount) + volumes = append(volumes, serverlessapi.AppVolume{MountPath: mount}) + } + return &volumes, nil +} + +// validateMountPath returns the cleaned path, or an error naming what is wrong +// with it. `seen` holds the already-accepted paths, cleaned. +func validateMountPath(raw string, seen []string) (string, error) { + // path.Clean, not filepath.Clean: this is a path inside the Linux sandbox + // the app runs in, whatever the machine the CLI runs on. + mount := path.Clean(raw) + + switch { + case !path.IsAbs(mount): + return "", fmt.Errorf("volume %q: must be an absolute path", raw) + case mount == "/": + return "", fmt.Errorf("volume %q: must not be the root directory", raw) + case len(mount) > maxVolumeMountPathLen: + return "", fmt.Errorf("volume %q: exceeds the %d character limit", raw, maxVolumeMountPathLen) + } + + for _, r := range mount { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || + (r >= '0' && r <= '9') || strings.ContainsRune(volumeMountPathExtra, r) { + continue + } + return "", fmt.Errorf("volume %q: contains unsupported character %q", raw, r) + } + + for _, component := range strings.Split(strings.TrimPrefix(mount, "/"), "/") { + if len(component) > maxVolumePathComponent { + return "", fmt.Errorf("volume %q: path component exceeds %d bytes", raw, maxVolumePathComponent) + } + } + + // Overlap, not just duplication: two volumes where one contains the other + // would bind-mount the same node directory into the sandbox twice, and there + // is no answer to which one owns the shared subtree. + for _, previous := range seen { + if previous == mount { + return "", fmt.Errorf("volume %q is listed twice", mount) + } + if strings.HasPrefix(mount, previous+"/") || strings.HasPrefix(previous, mount+"/") { + return "", fmt.Errorf("volume %q overlaps %q; declare only the outer path", mount, previous) + } + } + return mount, nil +} diff --git a/internal/cmd/serverless/volume_test.go b/internal/cmd/serverless/volume_test.go new file mode 100644 index 0000000..0752875 --- /dev/null +++ b/internal/cmd/serverless/volume_test.go @@ -0,0 +1,128 @@ +package serverless + +import ( + "strings" + "testing" +) + +// A representative mount path, repeated across the cases below. +const testMountPath = "/data/weights" + +func TestBuildVolumes(t *testing.T) { + got, err := buildVolumes([]string{"/root/.cache/huggingface", testMountPath}) + if err != nil { + t.Fatalf("buildVolumes: %v", err) + } + if got == nil || len(*got) != 2 { + t.Fatalf("expected 2 volumes, got %v", got) + } + if (*got)[0].MountPath != "/root/.cache/huggingface" { + t.Errorf("mountPath = %q", (*got)[0].MountPath) + } +} + +// No volumes must send nil rather than an empty array: the field is optional and +// an explicit [] is a different statement from saying nothing. +func TestBuildVolumes_NoneIsNil(t *testing.T) { + got, err := buildVolumes(nil) + if err != nil { + t.Fatalf("buildVolumes: %v", err) + } + if got != nil { + t.Errorf("expected nil for no volumes, got %v", *got) + } +} + +// The path is cleaned before it travels, so the server sees one spelling of a +// path however it was typed -- and so the overlap check below compares like +// with like. +func TestBuildVolumes_CleansPaths(t *testing.T) { + got, err := buildVolumes([]string{"/data//weights/"}) + if err != nil { + t.Fatalf("buildVolumes: %v", err) + } + if (*got)[0].MountPath != testMountPath { + t.Errorf("mountPath = %q, want /data/weights", (*got)[0].MountPath) + } +} + +func TestBuildVolumes_Rejects(t *testing.T) { + cases := []struct { + name string + paths []string + want string + }{ + { + name: "relative", + paths: []string{"cache/weights"}, + want: "absolute", + }, + { + name: "root", + paths: []string{"/"}, + want: "root directory", + }, + { + name: "unsupported character", + paths: []string{"/data/we ights"}, + want: "unsupported character", + }, + { + // The same path twice is a mistake, not a request for one volume. + name: "duplicate", + paths: []string{testMountPath, testMountPath}, + want: "listed twice", + }, + { + // The pair that motivates the whole check: one contains the other, + // so the same node directory would be mounted into the sandbox twice. + name: "overlapping", + paths: []string{"/data", testMountPath}, + want: "overlaps", + }, + { + // ...in either order. + name: "overlapping reversed", + paths: []string{testMountPath, "/data"}, + want: "overlaps", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := buildVolumes(tc.paths) + if err == nil { + t.Fatalf("expected an error for %v", tc.paths) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not mention %q", err, tc.want) + } + }) + } +} + +// A shared prefix is not containment: /data/weights-old sits beside +// /data/weights rather than inside it, so both are legitimate. +func TestBuildVolumes_SharedPrefixIsNotOverlap(t *testing.T) { + got, err := buildVolumes([]string{testMountPath, "/data/weights-old"}) + if err != nil { + t.Fatalf("buildVolumes rejected sibling paths: %v", err) + } + if len(*got) != 2 { + t.Errorf("expected both volumes, got %v", *got) + } +} + +func TestBuildVolumes_TooMany(t *testing.T) { + paths := make([]string, maxVolumes+1) + for i := range paths { + paths[i] = "/data/vol" + string(rune('a'+i%26)) + string(rune('a'+i/26)) + } + _, err := buildVolumes(paths) + if err == nil { + t.Fatal("expected an error past the volume limit") + } + if !strings.Contains(err.Error(), "at most") { + t.Errorf("error %q does not mention the limit", err) + } +}