From bc8ddc1fbf46ac3766f8951d31e810149aed43f9 Mon Sep 17 00:00:00 2001 From: Pawel Kuc Date: Wed, 16 Apr 2025 18:35:47 +0200 Subject: [PATCH 1/3] feat: unify baseConfig logic for apps.Config across app and project config Introduce shared apps.Config logic, in a form of baseConfig struct, that removes duplication between application and project config. As a result this automatically fixes missing features in project config such as missing scheduledjob and workerjob arguments and missing delete env support. Also introduce shared test helpers for baseConfig and job assertion logic. --- api/validation/config.go | 6 + create/application.go | 97 +++++---- create/application_test.go | 123 ++++++++---- create/project_config.go | 37 +--- create/project_config_test.go | 177 +++++++++++++--- internal/test/base_config.go | 96 +++++++++ update/application.go | 203 ++++++++++--------- update/application_test.go | 168 +++++++++++----- update/project_config.go | 27 +-- update/project_config_test.go | 368 +++++++++++++++++++++++++++++++--- 10 files changed, 961 insertions(+), 341 deletions(-) create mode 100644 internal/test/base_config.go diff --git a/api/validation/config.go b/api/validation/config.go index 91319784..a6526a1e 100644 --- a/api/validation/config.go +++ b/api/validation/config.go @@ -11,6 +11,12 @@ type ConfigValidator struct { Config apps.Config } +// TODO: remove local job validation logic in favor of webhook-based CRD validation +// We now rely on the Kubernetes admission webhook deployed for our CRD backend, +// which performs full validation of the config, including DeployJob. The removed +// logic only checked the job name and did not cover other important fields, +// making it insufficient for robust validation. + // Validate validates the config func (c ConfigValidator) Validate() error { if c.Config.DeployJob != nil { diff --git a/create/application.go b/create/application.go index 02836840..b988c5f5 100644 --- a/create/application.go +++ b/create/application.go @@ -35,23 +35,26 @@ const logPrintTimeout = 10 * time.Second // update/application.go. type applicationCmd struct { resourceCmd + baseConfig Git gitConfig `embed:"" prefix:"git-"` - Size *string `help:"Size of the app (defaults to \"${app_default_size}\")." placeholder:"${app_default_size}"` - Port *int32 `help:"Port the app is listening on (defaults to ${app_default_port})." placeholder:"${app_default_port}"` - Replicas *int32 `help:"Amount of replicas of the running app (defaults to ${app_default_replicas})." placeholder:"${app_default_replicas}"` Hosts []string `help:"Host names where the app can be accessed. If empty, the app will just be accessible on a generated host name on the deploio.app domain."` - BasicAuth *bool `help:"Enable/Disable basic authentication for the app (defaults to ${app_default_basic_auth})." placeholder:"${app_default_basic_auth}"` - Env map[string]string `help:"Environment variables which are passed to the app at runtime."` BuildEnv map[string]string `help:"Environment variables which are passed to the app build process."` - DeployJob deployJob `embed:"" prefix:"deploy-job-"` - WorkerJob workerJob `embed:"" prefix:"worker-job-"` - ScheduledJob scheduledJob `embed:"" prefix:"scheduled-job-"` GitInformationServiceURL string `help:"URL of the git information service." default:"https://git-info.deplo.io" env:"GIT_INFORMATION_SERVICE_URL" hidden:""` SkipRepoAccessCheck bool `help:"Skip the git repository access check" default:"false"` Debug bool `help:"Enable debug messages" default:"false"` Language string `help:"${app_language_help} Possible values: ${enum}" enum:"ruby,php,python,golang,nodejs,static," default:""` DockerfileBuild dockerfileBuild `embed:""` } +type baseConfig struct { + Size *string `help:"Size of the app (defaults to \"${app_default_size}\")." placeholder:"${app_default_size}"` + Port *int32 `help:"Port the app is listening on (defaults to ${app_default_port})." placeholder:"${app_default_port}"` + Replicas *int32 `help:"Amount of replicas of the running app (defaults to ${app_default_replicas})." placeholder:"${app_default_replicas}"` + BasicAuth *bool `help:"Enable/Disable basic authentication for the app (defaults to ${app_default_basic_auth})." placeholder:"${app_default_basic_auth}"` + Env map[string]string `help:"Environment variables which are passed to the app at runtime."` + DeployJob deployJob `embed:"" prefix:"deploy-job-"` + WorkerJob workerJob `embed:"" prefix:"worker-job-"` + ScheduledJob scheduledJob `embed:"" prefix:"scheduled-job-"` +} type gitConfig struct { URL string `required:"" help:"URL to the Git repository containing the app source. Both HTTPS and SSH formats are supported."` @@ -273,67 +276,75 @@ func spinnerMessage(msg, icon string, sleepTime time.Duration) error { return spinner.Stop() } -func (app *applicationCmd) config() apps.Config { - var deployJob *apps.DeployJob - - if len(app.DeployJob.Command) != 0 && len(app.DeployJob.Name) != 0 { - deployJob = &apps.DeployJob{ +func (job deployJob) applyUpdates(cfg *apps.Config) { + if len(job.Command) != 0 && len(job.Name) != 0 { + deployJob := &apps.DeployJob{ Job: apps.Job{ - Name: app.DeployJob.Name, - Command: app.DeployJob.Command, + Name: job.Name, + Command: job.Command, }, FiniteJob: apps.FiniteJob{ - Retries: ptr.To(app.DeployJob.Retries), - Timeout: &metav1.Duration{Duration: app.DeployJob.Timeout}, + Retries: ptr.To(job.Retries), + Timeout: &metav1.Duration{Duration: job.Timeout}, }, } + cfg.DeployJob = deployJob } +} - config := apps.Config{ - EnableBasicAuth: app.BasicAuth, - Env: util.EnvVarsFromMap(app.Env), - DeployJob: deployJob, - } - - if len(app.WorkerJob.Command) != 0 && len(app.WorkerJob.Name) != 0 { +func (job workerJob) applyUpdates(config *apps.Config) { + if len(job.Command) != 0 && len(job.Name) != 0 { workerJob := apps.WorkerJob{ Job: apps.Job{ - Name: app.WorkerJob.Name, - Command: app.WorkerJob.Command, + Name: job.Name, + Command: job.Command, }, } - if app.WorkerJob.Size != nil { - workerJob.Size = ptr.To(apps.ApplicationSize(*app.WorkerJob.Size)) + if job.Size != nil { + workerJob.Size = ptr.To(apps.ApplicationSize(*job.Size)) } config.WorkerJobs = append(config.WorkerJobs, workerJob) } +} - if len(app.ScheduledJob.Command) != 0 && len(app.ScheduledJob.Name) != 0 && len(app.ScheduledJob.Schedule) != 0 { +func (job scheduledJob) applyUpdates(config *apps.Config) { + if len(job.Command) != 0 && len(job.Name) != 0 && len(job.Schedule) != 0 { scheduledJob := apps.ScheduledJob{ FiniteJob: apps.FiniteJob{ - Retries: &app.ScheduledJob.Retries, - Timeout: &metav1.Duration{Duration: app.ScheduledJob.Timeout}, + Retries: &job.Retries, + Timeout: &metav1.Duration{Duration: job.Timeout}, }, Job: apps.Job{ - Name: app.ScheduledJob.Name, - Command: app.ScheduledJob.Command, + Name: job.Name, + Command: job.Command, }, - Schedule: app.ScheduledJob.Schedule, + Schedule: job.Schedule, } - if app.ScheduledJob.Size != nil { - scheduledJob.Size = ptr.To(apps.ApplicationSize(*app.ScheduledJob.Size)) + if job.Size != nil { + scheduledJob.Size = ptr.To(apps.ApplicationSize(*job.Size)) } config.ScheduledJobs = append(config.ScheduledJobs, scheduledJob) } +} + +func newConfig(bc baseConfig) apps.Config { + config := apps.Config{ + EnableBasicAuth: bc.BasicAuth, + Env: util.EnvVarsFromMap(bc.Env), + } + + bc.DeployJob.applyUpdates(&config) + bc.WorkerJob.applyUpdates(&config) + bc.ScheduledJob.applyUpdates(&config) - if app.Size != nil { - config.Size = apps.ApplicationSize(*app.Size) + if bc.Size != nil { + config.Size = apps.ApplicationSize(*bc.Size) } - if app.Port != nil { - config.Port = app.Port + if bc.Port != nil { + config.Port = bc.Port } - if app.Replicas != nil { - config.Replicas = app.Replicas + if bc.Replicas != nil { + config.Replicas = bc.Replicas } return config } @@ -357,7 +368,7 @@ func (app *applicationCmd) newApplication(project string) *apps.Application { }, }, Hosts: app.Hosts, - Config: app.config(), + Config: newConfig(app.baseConfig), BuildEnv: util.EnvVarsFromMap(app.BuildEnv), DockerfileBuild: apps.DockerfileBuild{ Enabled: app.DockerfileBuild.Enabled, diff --git a/create/application_test.go b/create/application_test.go index aa1f605b..afb96399 100644 --- a/create/application_test.go +++ b/create/application_test.go @@ -85,32 +85,19 @@ func TestApplication(t *testing.T) { SubPath: "/my/app", Revision: "superbug", }, - Size: ptr.To("mini"), + baseConfig: newBaseConfigCmdAllFields(), Hosts: []string{"custom.example.org", "custom2.example.org"}, - Port: ptr.To(int32(1337)), - Replicas: ptr.To(int32(42)), - BasicAuth: ptr.To(false), - Env: map[string]string{"hello": "world"}, BuildEnv: map[string]string{"BP_GO_TARGETS": "./cmd/web-server"}, - DeployJob: deployJob{Command: "date", Name: "print-date", Retries: 2, Timeout: time.Minute}, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) assert.Equal(t, cmd.Name, app.Name) assert.Equal(t, cmd.Git.URL, app.Spec.ForProvider.Git.URL) assert.Equal(t, cmd.Git.SubPath, app.Spec.ForProvider.Git.SubPath) assert.Equal(t, cmd.Git.Revision, app.Spec.ForProvider.Git.Revision) assert.Equal(t, cmd.Hosts, app.Spec.ForProvider.Hosts) - assert.Equal(t, apps.ApplicationSize(*cmd.Size), app.Spec.ForProvider.Config.Size) - assert.Equal(t, *cmd.Port, *app.Spec.ForProvider.Config.Port) - assert.Equal(t, *cmd.Replicas, *app.Spec.ForProvider.Config.Replicas) - assert.Equal(t, *cmd.BasicAuth, *app.Spec.ForProvider.Config.EnableBasicAuth) - assert.Equal(t, util.EnvVarsFromMap(cmd.Env), app.Spec.ForProvider.Config.Env) assert.Equal(t, util.EnvVarsFromMap(cmd.BuildEnv), app.Spec.ForProvider.BuildEnv) - assert.Equal(t, cmd.DeployJob.Command, app.Spec.ForProvider.Config.DeployJob.Command) - assert.Equal(t, cmd.DeployJob.Name, app.Spec.ForProvider.Config.DeployJob.Name) - assert.Equal(t, cmd.DeployJob.Timeout, app.Spec.ForProvider.Config.DeployJob.Timeout.Duration) - assert.Equal(t, cmd.DeployJob.Retries, *app.Spec.ForProvider.Config.DeployJob.Retries) assert.Nil(t, app.Spec.ForProvider.Git.Auth) }, }, @@ -120,14 +107,15 @@ func TestApplication(t *testing.T) { Wait: false, Name: "basic-auth", }, - Size: ptr.To("mini"), - BasicAuth: ptr.To(true), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + BasicAuth: ptr.To(true), + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) assert.Equal(t, cmd.Name, app.Name) - assert.Equal(t, apps.ApplicationSize(*cmd.Size), app.Spec.ForProvider.Config.Size) - assert.Equal(t, *cmd.BasicAuth, *app.Spec.ForProvider.Config.EnableBasicAuth) }, }, "with user/pass git auth": { @@ -144,6 +132,7 @@ func TestApplication(t *testing.T) { SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) auth := util.GitAuth{Username: cmd.Git.Username, Password: cmd.Git.Password} authSecret := auth.Secret(app) if err := apiClient.Get(ctx, api.ObjectName(authSecret), authSecret); err != nil { @@ -165,10 +154,13 @@ func TestApplication(t *testing.T) { Wait: false, Name: "ssh-key-auth", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) auth := util.GitAuth{SSHPrivateKey: cmd.Git.SSHPrivateKey} authSecret := auth.Secret(app) if err := apiClient.Get(ctx, api.ObjectName(authSecret), authSecret); err != nil { @@ -189,10 +181,13 @@ func TestApplication(t *testing.T) { Wait: false, Name: "ssh-key-auth-ed25519", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) auth := util.GitAuth{SSHPrivateKey: cmd.Git.SSHPrivateKey} authSecret := auth.Secret(app) if err := apiClient.Get(ctx, api.ObjectName(authSecret), authSecret); err != nil { @@ -213,10 +208,13 @@ func TestApplication(t *testing.T) { Wait: false, Name: "ssh-key-auth-from-file", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) auth := util.GitAuth{SSHPrivateKey: ptr.To("notused")} authSecret := auth.Secret(app) if err := apiClient.Get(ctx, api.ObjectName(authSecret), authSecret); err != nil { @@ -237,10 +235,13 @@ func TestApplication(t *testing.T) { Wait: false, Name: "ssh-key-auth-from-file-ed25519", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) auth := util.GitAuth{SSHPrivateKey: ptr.To("notused")} authSecret := auth.Secret(app) if err := apiClient.Get(ctx, api.ObjectName(authSecret), authSecret); err != nil { @@ -261,11 +262,14 @@ func TestApplication(t *testing.T) { Wait: false, Name: "ssh-key-auth-non-valid", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, SkipRepoAccessCheck: true, }, errorExpected: true, }, + // TODO: remove local job validation logic in favor of webhook-based CRD validation "deploy job empty command": { cmd: applicationCmd{ Git: gitConfig{ @@ -275,14 +279,17 @@ func TestApplication(t *testing.T) { Wait: false, Name: "deploy-job-empty-command", }, - Size: ptr.To("mini"), - DeployJob: deployJob{Command: "", Name: "print-date", Retries: 2, Timeout: time.Minute}, + baseConfig: baseConfig{ + Size: ptr.To("mini"), + DeployJob: deployJob{Command: "", Name: "print-date", Retries: 2, Timeout: time.Minute}, + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { - assert.Nil(t, app.Spec.ForProvider.Config.DeployJob) + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) }, }, + // TODO: remove local job validation logic in favor of webhook-based CRD validation "deploy job empty name": { cmd: applicationCmd{ Git: gitConfig{ @@ -292,12 +299,14 @@ func TestApplication(t *testing.T) { Wait: false, Name: "deploy-job-empty-name", }, - Size: ptr.To("mini"), - DeployJob: deployJob{Command: "date", Name: "", Retries: 2, Timeout: time.Minute}, + baseConfig: baseConfig{ + Size: ptr.To("mini"), + DeployJob: deployJob{Command: "date", Name: "", Retries: 2, Timeout: time.Minute}, + }, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { - assert.Nil(t, app.Spec.ForProvider.Config.DeployJob) + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) }, }, "git-information-service happy path": { @@ -311,7 +320,9 @@ func TestApplication(t *testing.T) { Wait: false, Name: "git-information-happy-path", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 200, @@ -328,11 +339,11 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) assert.Equal(t, cmd.Name, app.Name) assert.Equal(t, cmd.Git.URL, app.Spec.ForProvider.Git.URL) assert.Equal(t, cmd.Git.SubPath, app.Spec.ForProvider.Git.SubPath) assert.Equal(t, cmd.Git.Revision, app.Spec.ForProvider.Git.Revision) - assert.Equal(t, apps.ApplicationSize(*cmd.Size), app.Spec.ForProvider.Config.Size) assert.Nil(t, app.Spec.ForProvider.Git.Auth) }, }, @@ -347,7 +358,9 @@ func TestApplication(t *testing.T) { Wait: false, Name: "git-information-errors", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 200, @@ -368,7 +381,9 @@ func TestApplication(t *testing.T) { Wait: false, Name: "git-information-unknown-revision", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 200, @@ -397,7 +412,9 @@ func TestApplication(t *testing.T) { Wait: false, Name: "git-information-unknown-revision", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 501, @@ -416,7 +433,9 @@ func TestApplication(t *testing.T) { Wait: false, Name: "git-information-update-url-to-https", }, - Size: ptr.To("mini"), + baseConfig: baseConfig{ + Size: ptr.To("mini"), + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 200, @@ -432,11 +451,11 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { + assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) assert.Equal(t, cmd.Name, app.Name) assert.Equal(t, "https://github.com/ninech/doesnotexist.git", app.Spec.ForProvider.Git.URL) assert.Equal(t, cmd.Git.SubPath, app.Spec.ForProvider.Git.SubPath) assert.Equal(t, cmd.Git.Revision, app.Spec.ForProvider.Git.Revision) - assert.Equal(t, apps.ApplicationSize(*cmd.Size), app.Spec.ForProvider.Config.Size) assert.Nil(t, app.Spec.ForProvider.Git.Auth) }, }, @@ -466,6 +485,32 @@ func TestApplication(t *testing.T) { } } +func newBaseConfigCmdAllFields() baseConfig { + return baseConfig{ + Size: ptr.To(string(test.AppMini)), + Port: ptr.To(int32(1337)), + Replicas: ptr.To(int32(42)), + BasicAuth: ptr.To(true), + Env: map[string]string{"key1": "val1"}, + DeployJob: deployJob{ + Command: "exit 0", Name: "exit", + Retries: 1, Timeout: time.Minute * 5, + }, + WorkerJob: workerJob{ + Name: "do-stuff-1", + Command: "echo stuff1", + Size: ptr.To(string(test.AppStandard1)), + }, + ScheduledJob: scheduledJob{ + Command: "sleep 5; date", + Name: "scheduled-1", + Size: ptr.To(string(test.AppStandard1)), + Schedule: "* * * * *", + Retries: 1, Timeout: time.Minute * 5, + }, + } +} + func TestApplicationWait(t *testing.T) { cmd := applicationCmd{ resourceCmd: resourceCmd{ @@ -473,7 +518,9 @@ func TestApplicationWait(t *testing.T) { WaitTimeout: time.Second * 5, Name: "some-name", }, - BasicAuth: ptr.To(true), + baseConfig: baseConfig{ + BasicAuth: ptr.To(true), + }, SkipRepoAccessCheck: true, } project := test.DefaultProject diff --git a/create/project_config.go b/create/project_config.go index ec01f89a..6049db5f 100644 --- a/create/project_config.go +++ b/create/project_config.go @@ -5,20 +5,13 @@ import ( apps "github.com/ninech/apis/apps/v1alpha1" "github.com/ninech/nctl/api" - "github.com/ninech/nctl/api/util" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/ptr" ) // all fields need to be pointers so we can detect if they have been set by // the user. type configCmd struct { - Size string `help:"Size of the app."` - Port *int32 `help:"Port the app is listening on."` - Replicas *int32 `help:"Amount of replicas of the running app."` - Env *map[string]string `help:"Environment variables which are passed to the app at runtime."` - BasicAuth *bool `help:"Enable/Disable basic authentication for applications."` - DeployJob deployJob `embed:"" prefix:"deploy-job-"` + baseConfig } func (cmd *configCmd) Run(ctx context.Context, client *api.Client) error { @@ -28,25 +21,6 @@ func (cmd *configCmd) Run(ctx context.Context, client *api.Client) error { } func (cmd *configCmd) newProjectConfig(namespace string) *apps.ProjectConfig { - env := apps.EnvVars{} - if cmd.Env != nil { - env = util.EnvVarsFromMap(*cmd.Env) - } - - var deployJob *apps.DeployJob - if len(cmd.DeployJob.Command) != 0 && len(cmd.DeployJob.Name) != 0 { - deployJob = &apps.DeployJob{ - Job: apps.Job{ - Name: cmd.DeployJob.Name, - Command: cmd.DeployJob.Command, - }, - FiniteJob: apps.FiniteJob{ - Retries: ptr.To(cmd.DeployJob.Retries), - Timeout: &metav1.Duration{Duration: cmd.DeployJob.Timeout}, - }, - } - } - return &apps.ProjectConfig{ ObjectMeta: metav1.ObjectMeta{ Name: namespace, @@ -54,14 +28,7 @@ func (cmd *configCmd) newProjectConfig(namespace string) *apps.ProjectConfig { }, Spec: apps.ProjectConfigSpec{ ForProvider: apps.ProjectConfigParameters{ - Config: apps.Config{ - Size: apps.ApplicationSize(cmd.Size), - Replicas: cmd.Replicas, - Port: cmd.Port, - Env: env, - EnableBasicAuth: cmd.BasicAuth, - DeployJob: deployJob, - }, + Config: newConfig(cmd.baseConfig), }, }, } diff --git a/create/project_config_test.go b/create/project_config_test.go index 2d338125..058925d1 100644 --- a/create/project_config_test.go +++ b/create/project_config_test.go @@ -3,13 +3,13 @@ package create import ( "context" "testing" - "time" apps "github.com/ninech/apis/apps/v1alpha1" "github.com/ninech/nctl/api" "github.com/ninech/nctl/api/util" "github.com/ninech/nctl/internal/test" "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" ) @@ -28,57 +28,38 @@ func TestProjectConfig(t *testing.T) { }{ "all fields set": { cmd: configCmd{ - Size: string(test.AppMini), - Port: ptr.To(int32(1337)), - Replicas: ptr.To(int32(42)), - Env: &map[string]string{"key1": "val1"}, - BasicAuth: ptr.To(true), - DeployJob: deployJob{ - Command: "exit 0", Name: "exit", - Retries: 1, Timeout: time.Minute * 5, - }, + baseConfig: newBaseConfigCmdAllFields(), }, project: "namespace-1", checkConfig: func(t *testing.T, cmd configCmd, cfg *apps.ProjectConfig) { + assertBaseConfig(t, cmd.baseConfig, cfg.Spec.ForProvider.Config) assert.Equal(t, apiClient.Project, cfg.Name) - assert.Equal(t, apps.ApplicationSize(cmd.Size), cfg.Spec.ForProvider.Config.Size) - assert.Equal(t, *cmd.Port, *cfg.Spec.ForProvider.Config.Port) - assert.Equal(t, *cmd.Replicas, *cfg.Spec.ForProvider.Config.Replicas) - assert.Equal(t, *cmd.BasicAuth, *cfg.Spec.ForProvider.Config.EnableBasicAuth) - assert.Equal(t, util.EnvVarsFromMap(*cmd.Env), cfg.Spec.ForProvider.Config.Env) - assert.Equal(t, cmd.DeployJob.Command, cfg.Spec.ForProvider.Config.DeployJob.Command) - assert.Equal(t, cmd.DeployJob.Name, cfg.Spec.ForProvider.Config.DeployJob.Name) - assert.Equal(t, cmd.DeployJob.Timeout, cfg.Spec.ForProvider.Config.DeployJob.Timeout.Duration) - assert.Equal(t, cmd.DeployJob.Retries, *cfg.Spec.ForProvider.Config.DeployJob.Retries) }, }, "some fields not set": { cmd: configCmd{ - Size: string(test.AppMicro), - Replicas: ptr.To(int32(1)), + baseConfig: baseConfig{ + Size: ptr.To(string(test.AppMicro)), + Replicas: ptr.To(int32(1)), + WorkerJob: workerJob{ + Name: "do-stuff-2", + Command: "echo stuff2", + Size: ptr.To(string(test.AppSizeNotSet)), + }, + }, }, project: "namespace-2", checkConfig: func(t *testing.T, cmd configCmd, cfg *apps.ProjectConfig) { + assertBaseConfig(t, cmd.baseConfig, cfg.Spec.ForProvider.Config) assert.Equal(t, apiClient.Project, cfg.Name) - assert.Equal(t, apps.ApplicationSize(cmd.Size), cfg.Spec.ForProvider.Config.Size) - assert.Nil(t, cfg.Spec.ForProvider.Config.Port) - assert.Nil(t, cfg.Spec.ForProvider.Config.EnableBasicAuth) - assert.Equal(t, *cmd.Replicas, *cfg.Spec.ForProvider.Config.Replicas) - assert.Empty(t, cfg.Spec.ForProvider.Config.Env) - assert.Nil(t, cfg.Spec.ForProvider.Config.DeployJob) }, }, "all fields not set": { cmd: configCmd{}, project: "namespace-3", checkConfig: func(t *testing.T, cmd configCmd, cfg *apps.ProjectConfig) { + assertBaseConfig(t, cmd.baseConfig, cfg.Spec.ForProvider.Config) assert.Equal(t, apiClient.Project, cfg.Name) - assert.Equal(t, test.AppSizeNotSet, cfg.Spec.ForProvider.Config.Size) - assert.Nil(t, cfg.Spec.ForProvider.Config.Port) - assert.Nil(t, cfg.Spec.ForProvider.Config.Replicas) - assert.Empty(t, cfg.Spec.ForProvider.Config.Env) - assert.Nil(t, cfg.Spec.ForProvider.Config.EnableBasicAuth) - assert.Nil(t, cfg.Spec.ForProvider.Config.DeployJob) }, }, } @@ -101,3 +82,133 @@ func TestProjectConfig(t *testing.T) { }) } } + +// assertBaseConfig verifies that apps.Config fields were correctly updated +// based on the provided CLI config. It checks both modified fields and ensures +// that unspecified fields remain unchanged. +func assertBaseConfig(t *testing.T, cmd baseConfig, got apps.Config) { + t.Helper() + + assertBaseConfigCore(t, cmd, got) + assertBaseConfigJobs(t, cmd, got) +} + +// assertBaseConfigCore asserts that the "core" flags (baseConfig) in CLI config +// ended up in the apps.Config, and that Env collection is updated correctly. It +// also ensures that unspecified fields remain default. +func assertBaseConfigCore(t *testing.T, cmd baseConfig, got apps.Config) { + t.Helper() + + if cmd.Size != nil { + assert.Equal(t, apps.ApplicationSize(*cmd.Size), got.Size, "size") + } else { + assert.Equal(t, test.AppSizeNotSet, got.Size, "size (default)") + } + if cmd.Port != nil { + assert.Equal(t, *cmd.Port, *got.Port, "port") + } else { + assert.Nil(t, got.Port, "port (default)") + } + if cmd.Replicas != nil { + assert.Equal(t, *cmd.Replicas, *got.Replicas, "replicas") + } else { + assert.Nil(t, got.Replicas, "replicas (default)") + } + if cmd.BasicAuth != nil { + assert.Equal(t, *cmd.BasicAuth, *got.EnableBasicAuth, "basic auth") + } else { + assert.Nil(t, got.EnableBasicAuth, "basic auth (default)") + } + if cmd.Env != nil { + expectedEnv := util.EnvVarsFromMap(cmd.Env) + assert.Equal(t, expectedEnv, got.Env, "env vars") + } else { + assert.Empty(t, got.Env, "env vars (default)") + } +} + +// assertBaseConfigJobs provides a default logic for jobs assert. It checks +// both modified fields and ensures that unspecified fields remain default. +func assertBaseConfigJobs(t *testing.T, cmd baseConfig, got apps.Config) { + t.Helper() + + wantDeployJobs := deployJobsFromCmdNormalized(cmd.DeployJob) + gotDeployJobs := test.PtrToSlice(got.DeployJob, func(d *apps.DeployJob) apps.DeployJob { return *d }) + test.AssertJobsEqual(t, wantDeployJobs, gotDeployJobs, func(d apps.DeployJob) test.DeployJobKey { return test.ToDeployJobKey(&d) }) + wantWorkerJobs := test.PtrToSlice(workerJobPtr(cmd.WorkerJob), workerJobFromCmd) + gotWorkerJobs := test.NormalizeSlice(got.WorkerJobs) + test.AssertJobsEqual(t, wantWorkerJobs, gotWorkerJobs, test.ToWorkerJobKey) + wantScheduledJobs := test.PtrToSlice(scheduledJobPtr(cmd.ScheduledJob), scheduledJobFromCmd) + gotScheduledJobs := test.NormalizeSlice(got.ScheduledJobs) + test.AssertJobsEqual(t, wantScheduledJobs, gotScheduledJobs, test.ToScheduledJobKey) +} + +// deployJobsFromCmdNormalized converts the CLI deployJob into the slice of CRD DeployJob. +func deployJobsFromCmdNormalized(j deployJob) []apps.DeployJob { + if len(j.Command) == 0 || len(j.Name) == 0 { + return nil + } + dj := apps.DeployJob{ + Job: apps.Job{ + Name: j.Name, + Command: j.Command, + }, + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(j.Retries), + Timeout: &metav1.Duration{Duration: j.Timeout}, + }, + } + return []apps.DeployJob{dj} +} + +// workerJobFromCmd converts the CLI workerJob into the CRD WorkerJob. +func workerJobFromCmd(j *workerJob) apps.WorkerJob { + if j == nil { + return apps.WorkerJob{} + } + return apps.WorkerJob{ + Job: apps.Job{ + Name: j.Name, + Command: j.Command, + }, + Size: ptr.To(apps.ApplicationSize(ptr.Deref(j.Size, ""))), + } +} + +// scheduledJobFromCmd turns the CLI representation into the form that finally +// ends up on the ProjectConfig after the mapping/conversion logic. +func scheduledJobFromCmd(j *scheduledJob) apps.ScheduledJob { + if j == nil { + return apps.ScheduledJob{} + } + return apps.ScheduledJob{ + Job: apps.Job{ + Name: j.Name, + Command: j.Command, + }, + Size: ptr.To(apps.ApplicationSize(ptr.Deref(j.Size, ""))), + Schedule: j.Schedule, + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(j.Retries), + Timeout: &metav1.Duration{Duration: j.Timeout}, + }, + } +} + +// workerJobPtr returns nil if no or invalid --worker-job- flags were passed, +// or &j otherwise. +func workerJobPtr(j workerJob) *workerJob { + if len(j.Command) == 0 || len(j.Name) == 0 { + return nil + } + return &j +} + +// scheduledJobPtr returns nil if no or invalid --scheduled-job- flags were passed, +// or &j otherwise. +func scheduledJobPtr(j scheduledJob) *scheduledJob { + if len(j.Command) == 0 || len(j.Name) == 0 { + return nil + } + return &j +} diff --git a/internal/test/base_config.go b/internal/test/base_config.go new file mode 100644 index 00000000..2b7ab226 --- /dev/null +++ b/internal/test/base_config.go @@ -0,0 +1,96 @@ +package test + +import ( + "testing" + "time" + + apps "github.com/ninech/apis/apps/v1alpha1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" +) + +// DeployJobKey defines deploy job comparable key +type DeployJobKey struct { + Name, Command string + Timeout time.Duration +} + +// ToDeployJobKey outputs deploy job comparable key +func ToDeployJobKey(j *apps.DeployJob) DeployJobKey { + return DeployJobKey{ + Name: j.Job.Name, + Command: j.Job.Command, + Timeout: ptr.Deref(j.Timeout, metav1.Duration{}).Duration, + } +} + +// WorkerJobKey defines worker job comparable key +type WorkerJobKey struct { + Name, Command string + Size apps.ApplicationSize +} + +// ToWorkerJobKey outputs worker job comparable key +func ToWorkerJobKey(j apps.WorkerJob) WorkerJobKey { + return WorkerJobKey{ + Name: j.Job.Name, + Command: j.Job.Command, + Size: ptr.Deref(j.Size, ""), + } +} + +// ScheduledJobKey defines scheduled job comparable key +type ScheduledJobKey struct { + Name, Command, Schedule string + Size apps.ApplicationSize + Retries int32 + Timeout time.Duration +} + +// ToScheduledJobKey outputs scheduled job comparable key +func ToScheduledJobKey(j apps.ScheduledJob) ScheduledJobKey { + return ScheduledJobKey{ + Name: j.Job.Name, + Command: j.Job.Command, + Schedule: j.Schedule, + Size: ptr.Deref(j.Size, ""), + Retries: ptr.Deref(j.Retries, 0), + Timeout: ptr.Deref(j.Timeout, metav1.Duration{}).Duration, + } +} + +// AssertJobsEqual generically checks that two slices of T contain exactly the +// same elements (order-independent), by mapping each element through toKey - a +// comparable key type +func AssertJobsEqual[T any, K comparable](t *testing.T, want, got []T, toKey func(T) K) { + t.Helper() + + // record the exact frequency of each key. + // That way we assert not only "this job exists' but also "and it exists exactly N times." + count := func(list []T) map[K]int { + m := make(map[K]int, len(list)) + for _, x := range list { + m[toKey(x)]++ + } + return m + } + + assert.Equal(t, count(want), count(got)) +} + +// PtrToSlice converts any *S into a []D by calling conv, or returns nil if src==nil. +func PtrToSlice[S any, D any](src *S, conv func(*S) D) []D { + if src == nil { + return nil + } + return []D{conv(src)} +} + +// NormalizeSlice turns any []T into nil if empty, else returns it unchanged. +func NormalizeSlice[T any](items []T) []T { + if len(items) == 0 { + return nil + } + return items +} diff --git a/update/application.go b/update/application.go index 119d6174..08d9064b 100644 --- a/update/application.go +++ b/update/application.go @@ -27,34 +27,38 @@ const BuildTrigger = "BUILD_TRIGGER" // the user. type applicationCmd struct { resourceCmd - Git *gitConfig `embed:"" prefix:"git-"` - Size *string `help:"Size of the app."` - Port *int32 `help:"Port the app is listening on."` - Replicas *int32 `help:"Amount of replicas of the running app."` - Hosts *[]string `help:"Host names where the application can be accessed. If empty, the application will just be accessible on a generated host name on the deploio.app domain."` - BasicAuth *bool `help:"Enable/Disable basic authentication for the application."` - ChangeBasicAuthPassword *bool `help:"Generate a new basic auth password."` - Env map[string]string `help:"Environment variables which are passed to the app at runtime."` - DeleteEnv *[]string `help:"Runtime environment variables names which are to be deleted."` - BuildEnv map[string]string `help:"Environment variables names which are passed to the app build process."` - DeleteBuildEnv *[]string `help:"Build environment variables which are to be deleted."` + baseConfig + Git *gitConfig `embed:"" prefix:"git-"` + Hosts *[]string `help:"Host names where the application can be accessed. If empty, the application will just be accessible on a generated host name on the deploio.app domain."` + ChangeBasicAuthPassword *bool `help:"Generate a new basic auth password."` + BuildEnv map[string]string `help:"Environment variables names which are passed to the app build process."` + DeleteBuildEnv *[]string `help:"Build environment variables which are to be deleted."` + RetryRelease *bool `help:"Retries release for the application." placeholder:"false"` + RetryBuild *bool `help:"Retries build for the application if set to true." placeholder:"false"` + Pause *bool `help:"Pauses the application if set to true. Stops all costs." placeholder:"false"` + GitInformationServiceURL string `help:"URL of the git information service." default:"https://git-info.deplo.io" env:"GIT_INFORMATION_SERVICE_URL" hidden:""` + SkipRepoAccessCheck bool `help:"Skip the git repository access check" default:"false"` + Debug bool `help:"Enable debug messages" default:"false"` + Language *string `help:"${app_language_help} Possible values: ${enum}" enum:"ruby,php,python,golang,nodejs,static,"` + DockerfileBuild dockerfileBuild `embed:""` +} + +type baseConfig struct { + Size *string `help:"Size of the app."` + Port *int32 `help:"Port the app is listening on."` + Replicas *int32 `help:"Amount of replicas of the running app."` + BasicAuth *bool `help:"Enable/Disable basic authentication for the application."` + Env map[string]string `help:"Environment variables which are passed to the app at runtime."` + DeleteEnv *[]string `help:"Runtime environment variables names which are to be deleted."` // DeployJob, ScheduledJob and WorkerJob are embedded pointers to // structs. Due to the usage of kong these pointers will never be `nil`. // So checking for `nil` values can not be used to find out if some of // the struct fields have been set. - DeployJob *deployJob `embed:"" prefix:"deploy-job-"` - WorkerJob *workerJob `embed:"" prefix:"worker-job-"` - ScheduledJob *scheduledJob `embed:"" prefix:"scheduled-job-"` - DeleteWorkerJob *string `help:"Delete a worker job by name"` - DeleteScheduledJob *string `help:"Delete a scheduled job by name"` - RetryRelease *bool `help:"Retries release for the application." placeholder:"false"` - RetryBuild *bool `help:"Retries build for the application if set to true." placeholder:"false"` - Pause *bool `help:"Pauses the application if set to true. Stops all costs." placeholder:"false"` - GitInformationServiceURL string `help:"URL of the git information service." default:"https://git-info.deplo.io" env:"GIT_INFORMATION_SERVICE_URL" hidden:""` - SkipRepoAccessCheck bool `help:"Skip the git repository access check" default:"false"` - Debug bool `help:"Enable debug messages" default:"false"` - Language *string `help:"${app_language_help} Possible values: ${enum}" enum:"ruby,php,python,golang,nodejs,static,"` - DockerfileBuild dockerfileBuild `embed:""` + DeployJob *deployJob `embed:"" prefix:"deploy-job-"` + WorkerJob *workerJob `embed:"" prefix:"worker-job-"` + ScheduledJob *scheduledJob `embed:"" prefix:"scheduled-job-"` + DeleteWorkerJob *string `help:"Delete a worker job by name"` + DeleteScheduledJob *string `help:"Delete a scheduled job by name"` } type gitConfig struct { @@ -214,6 +218,8 @@ func (cmd *applicationCmd) Run(ctx context.Context, client *api.Client) error { } func (cmd *applicationCmd) applyUpdates(app *apps.Application) { + cmd.baseConfig.applyUpdates(&app.Spec.ForProvider.Config) + if cmd.Git != nil { if cmd.Git.URL != nil { app.Spec.ForProvider.Git.URL = *cmd.Git.URL @@ -225,57 +231,21 @@ func (cmd *applicationCmd) applyUpdates(app *apps.Application) { app.Spec.ForProvider.Git.Revision = *cmd.Git.Revision } } - if cmd.Size != nil { - newSize := apps.ApplicationSize(*cmd.Size) - app.Spec.ForProvider.Config.Size = newSize - } - if cmd.Port != nil { - app.Spec.ForProvider.Config.Port = cmd.Port - } - if cmd.Replicas != nil { - app.Spec.ForProvider.Config.Replicas = cmd.Replicas + if cmd.RetryRelease != nil && *cmd.RetryRelease { + runtimeEnv := make(map[string]string) + runtimeEnv[ReleaseTrigger] = triggerTimestamp() + app.Spec.ForProvider.Config.Env = util.UpdateEnvVars(app.Spec.ForProvider.Config.Env, runtimeEnv, nil) } if cmd.Hosts != nil { app.Spec.ForProvider.Hosts = *cmd.Hosts } - if cmd.BasicAuth != nil { - app.Spec.ForProvider.Config.EnableBasicAuth = cmd.BasicAuth - } if cmd.ChangeBasicAuthPassword != nil { app.Spec.ForProvider.BasicAuthPasswordChange = ptr.To(metav1.Now()) } - if cmd.DeployJob != nil { - cmd.DeployJob.applyUpdates(&app.Spec.ForProvider.Config) - } - if cmd.WorkerJob != nil && cmd.WorkerJob.changesGiven() { - cmd.WorkerJob.applyUpdates(&app.Spec.ForProvider.Config) - } - if cmd.DeleteWorkerJob != nil { - deleteWorkerJob(*cmd.DeleteWorkerJob, &app.Spec.ForProvider.Config) - } - if cmd.ScheduledJob != nil && cmd.ScheduledJob.changesGiven() { - cmd.ScheduledJob.applyUpdates(&app.Spec.ForProvider.Config) - } - if cmd.DeleteScheduledJob != nil { - deleteScheduledJob(*cmd.DeleteScheduledJob, &app.Spec.ForProvider.Config) - } if cmd.Language != nil { app.Spec.ForProvider.Language = apps.Language(*cmd.Language) } - runtimeEnv := make(map[string]string) - if cmd.Env != nil { - runtimeEnv = cmd.Env - } - if cmd.RetryRelease != nil && *cmd.RetryRelease { - runtimeEnv[ReleaseTrigger] = triggerTimestamp() - } - var delEnv []string - if cmd.DeleteEnv != nil { - delEnv = *cmd.DeleteEnv - } - app.Spec.ForProvider.Config.Env = util.UpdateEnvVars(app.Spec.ForProvider.Config.Env, runtimeEnv, delEnv) - buildEnv := make(map[string]string) if cmd.BuildEnv != nil { buildEnv = cmd.BuildEnv @@ -297,13 +267,54 @@ func (cmd *applicationCmd) applyUpdates(app *apps.Application) { app.Spec.ForProvider.DockerfileBuild.DockerfilePath = *cmd.DockerfileBuild.Path warnIfDockerfileNotEnabled(app, "path") } - if cmd.DockerfileBuild.BuildContext != nil { app.Spec.ForProvider.DockerfileBuild.BuildContext = *cmd.DockerfileBuild.BuildContext warnIfDockerfileNotEnabled(app, "build context") } } +func (bc baseConfig) applyUpdates(config *apps.Config) { + if bc.BasicAuth != nil { + config.EnableBasicAuth = bc.BasicAuth + } + runtimeEnv := make(map[string]string) + if bc.Env != nil { + runtimeEnv = bc.Env + } + var delEnv []string + if bc.DeleteEnv != nil { + delEnv = *bc.DeleteEnv + } + config.Env = util.UpdateEnvVars(config.Env, runtimeEnv, delEnv) + + if bc.DeployJob != nil { + bc.DeployJob.applyUpdates(config) + } + if bc.WorkerJob != nil && bc.WorkerJob.changesGiven() { + bc.WorkerJob.applyUpdates(config) + } + if bc.DeleteWorkerJob != nil { + deleteWorkerJob(*bc.DeleteWorkerJob, config) + } + if bc.ScheduledJob != nil && bc.ScheduledJob.changesGiven() { + bc.ScheduledJob.applyUpdates(config) + } + if bc.DeleteScheduledJob != nil { + deleteScheduledJob(*bc.DeleteScheduledJob, config) + } + + if bc.Size != nil { + newSize := apps.ApplicationSize(*bc.Size) + config.Size = newSize + } + if bc.Port != nil { + config.Port = bc.Port + } + if bc.Replicas != nil { + config.Replicas = bc.Replicas + } +} + func triggerTimestamp() string { return time.Now().UTC().Format(time.RFC3339) } @@ -342,25 +353,23 @@ func (job workerJob) applyUpdates(cfg *apps.Config) { format.PrintWarningf("you need to pass a job name to update the command or size\n") return } + applyToJob := func(j *apps.WorkerJob) { + if job.Command != nil { + j.Command = *job.Command + } + if job.Size != nil { + j.Size = ptr.To(apps.ApplicationSize(*job.Size)) + } + } for i := range cfg.WorkerJobs { if cfg.WorkerJobs[i].Name == *job.Name { - if job.Command != nil { - cfg.WorkerJobs[i].Command = *job.Command - } - if job.Size != nil { - cfg.WorkerJobs[i].Size = ptr.To(apps.ApplicationSize(*job.Size)) - } + applyToJob(&cfg.WorkerJobs[i]) return } } newJob := apps.WorkerJob{Job: apps.Job{Name: *job.Name}} - if job.Command != nil { - newJob.Command = *job.Command - } - if job.Size != nil { - newJob.Size = ptr.To(apps.ApplicationSize(*job.Size)) - } + applyToJob(&newJob) cfg.WorkerJobs = append(cfg.WorkerJobs, newJob) } @@ -384,34 +393,32 @@ func (job scheduledJob) applyUpdates(cfg *apps.Config) { return } + applyToJob := func(j *apps.ScheduledJob) { + if job.Command != nil { + j.Command = *job.Command + } + if job.Size != nil { + j.Size = ptr.To(apps.ApplicationSize(*job.Size)) + } + if job.Schedule != nil { + j.Schedule = *job.Schedule + } + if job.Retries != nil { + j.Retries = job.Retries + } + if job.Timeout != nil { + j.Timeout = &metav1.Duration{Duration: *job.Timeout} + } + } for i := range cfg.ScheduledJobs { if cfg.ScheduledJobs[i].Name == *job.Name { - if job.Command != nil { - cfg.ScheduledJobs[i].Command = *job.Command - } - if job.Size != nil { - cfg.ScheduledJobs[i].Size = ptr.To(apps.ApplicationSize(*job.Size)) - } - if job.Schedule != nil { - cfg.ScheduledJobs[i].Schedule = *job.Schedule - } - if job.Retries != nil { - cfg.DeployJob.Retries = job.Retries - } - if job.Timeout != nil { - cfg.DeployJob.Timeout = &metav1.Duration{Duration: *job.Timeout} - } + applyToJob(&cfg.ScheduledJobs[i]) return } } newJob := apps.ScheduledJob{Job: apps.Job{Name: *job.Name}} - if job.Command != nil { - newJob.Command = *job.Command - } - if job.Size != nil { - newJob.Size = ptr.To(apps.ApplicationSize(*job.Size)) - } + applyToJob(&newJob) cfg.ScheduledJobs = append(cfg.ScheduledJobs, newJob) } diff --git a/update/application_test.go b/update/application_test.go index ac8e04c4..dc5dec91 100644 --- a/update/application_test.go +++ b/update/application_test.go @@ -53,16 +53,6 @@ func TestApplication(t *testing.T) { Port: ptr.To(int32(1337)), Env: util.EnvVarsFromMap(map[string]string{"foo": "bar"}), EnableBasicAuth: ptr.To(false), - DeployJob: &apps.DeployJob{ - Job: apps.Job{ - Command: "date", - Name: "print-date", - }, - FiniteJob: apps.FiniteJob{ - Retries: ptr.To(int32(2)), - Timeout: &metav1.Duration{Duration: time.Minute}, - }, - }, }, BuildEnv: util.EnvVarsFromMap(map[string]string{"BP_ENVIRONMENT_VARIABLE": "some-value"}), }, @@ -84,10 +74,12 @@ func TestApplication(t *testing.T) { resourceCmd: resourceCmd{ Name: existingApp.Name, }, - Port: ptr.To(int32(1234)), + baseConfig: baseConfig{ + Port: ptr.To(int32(1234)), + }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { - assert.Equal(t, *cmd.Port, *updated.Spec.ForProvider.Config.Port) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "port is unchanged when updating unrelated field": { @@ -96,11 +88,12 @@ func TestApplication(t *testing.T) { resourceCmd: resourceCmd{ Name: existingApp.Name, }, - Size: ptr.To("newsize"), + baseConfig: baseConfig{ + Size: ptr.To("newsize"), + }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { - assert.Equal(t, *orig.Spec.ForProvider.Config.Port, *updated.Spec.ForProvider.Config.Port) - assert.NotEqual(t, orig.Spec.ForProvider.Config.Size, updated.Spec.ForProvider.Config.Size) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "all field updates": { @@ -114,34 +107,18 @@ func TestApplication(t *testing.T) { SubPath: ptr.To("new/path"), Revision: ptr.To("some-change"), }, - Size: ptr.To("newsize"), - Port: ptr.To(int32(1234)), - Replicas: ptr.To(int32(999)), - Hosts: &[]string{"one.example.org", "two.example.org"}, - Env: map[string]string{"bar": "zoo"}, - BuildEnv: map[string]string{"BP_GO_TARGETS": "./cmd/web-server"}, - BasicAuth: ptr.To(true), - DeployJob: &deployJob{ - Command: ptr.To("exit 0"), Name: ptr.To("exit"), - Retries: ptr.To(int32(1)), Timeout: ptr.To(time.Minute * 5), - }, + baseConfig: newBaseConfigCmdAllFields(), + Hosts: &[]string{"one.example.org", "two.example.org"}, + BuildEnv: map[string]string{"BP_GO_TARGETS": "./cmd/web-server"}, SkipRepoAccessCheck: true, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.Equal(t, *cmd.Git.URL, updated.Spec.ForProvider.Git.URL) assert.Equal(t, *cmd.Git.SubPath, updated.Spec.ForProvider.Git.SubPath) assert.Equal(t, *cmd.Git.Revision, updated.Spec.ForProvider.Git.Revision) - assert.Equal(t, apps.ApplicationSize(*cmd.Size), updated.Spec.ForProvider.Config.Size) - assert.Equal(t, *cmd.Port, *updated.Spec.ForProvider.Config.Port) - assert.Equal(t, *cmd.Replicas, *updated.Spec.ForProvider.Config.Replicas) - assert.Equal(t, *cmd.BasicAuth, *updated.Spec.ForProvider.Config.EnableBasicAuth) assert.Equal(t, *cmd.Hosts, updated.Spec.ForProvider.Hosts) - assert.Equal(t, util.UpdateEnvVars(existingApp.Spec.ForProvider.Config.Env, cmd.Env, nil), updated.Spec.ForProvider.Config.Env) assert.Equal(t, util.UpdateEnvVars(existingApp.Spec.ForProvider.BuildEnv, cmd.BuildEnv, nil), updated.Spec.ForProvider.BuildEnv) - assert.Equal(t, *cmd.DeployJob.Command, updated.Spec.ForProvider.Config.DeployJob.Command) - assert.Equal(t, *cmd.DeployJob.Name, updated.Spec.ForProvider.Config.DeployJob.Name) - assert.Equal(t, *cmd.DeployJob.Timeout, updated.Spec.ForProvider.Config.DeployJob.Timeout.Duration) - assert.Equal(t, *cmd.DeployJob.Retries, *updated.Spec.ForProvider.Config.DeployJob.Retries) // Retry Release/Build should be not set by default: assert.Nil(t, util.EnvVarByName(updated.Spec.ForProvider.Config.Env, ReleaseTrigger)) assert.Nil(t, util.EnvVarByName(updated.Spec.ForProvider.BuildEnv, BuildTrigger)) @@ -153,10 +130,12 @@ func TestApplication(t *testing.T) { resourceCmd: resourceCmd{ Name: existingApp.Name, }, - DeleteEnv: &[]string{"foo"}, + baseConfig: baseConfig{ + DeleteEnv: &[]string{"foo"}, + }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { - assert.Empty(t, updated.Spec.ForProvider.Config.Env) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.NotEmpty(t, updated.Spec.ForProvider.BuildEnv) }, }, @@ -166,12 +145,12 @@ func TestApplication(t *testing.T) { resourceCmd: resourceCmd{ Name: existingApp.Name, }, - Env: map[string]string{"bar1": "zoo", "bar2": "foo"}, + baseConfig: baseConfig{ + Env: map[string]string{"bar1": "zoo", "bar2": "foo"}, + }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { - assert.Contains(t, updated.Spec.ForProvider.Config.Env, apps.EnvVar{Name: "bar1", Value: "zoo"}) - assert.Contains(t, updated.Spec.ForProvider.Config.Env, apps.EnvVar{Name: "bar2", Value: "foo"}) - assert.Contains(t, updated.Spec.ForProvider.Config.Env, apps.EnvVar{Name: "foo", Value: "bar"}) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "reset build env variable": { @@ -183,8 +162,8 @@ func TestApplication(t *testing.T) { DeleteBuildEnv: &[]string{"BP_ENVIRONMENT_VARIABLE"}, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.Empty(t, updated.Spec.ForProvider.BuildEnv) - assert.NotEmpty(t, updated.Spec.ForProvider.Config.Env) }, }, "change basic auth password": { @@ -196,6 +175,7 @@ func TestApplication(t *testing.T) { ChangeBasicAuthPassword: ptr.To(true), }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.NotNil(t, updated.Spec.ForProvider.BasicAuthPasswordChange) }, }, @@ -227,6 +207,9 @@ func TestApplication(t *testing.T) { }, }, }, + checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, checkSecret: func(t *testing.T, cmd applicationCmd, authSecret *corev1.Secret) { assert.Equal(t, *cmd.Git.Username, string(authSecret.Data[util.UsernameSecretKey])) assert.Equal(t, *cmd.Git.Password, string(authSecret.Data[util.PasswordSecretKey])) @@ -259,6 +242,9 @@ func TestApplication(t *testing.T) { }, }, }, + checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, checkSecret: func(t *testing.T, cmd applicationCmd, authSecret *corev1.Secret) { assert.Equal(t, strings.TrimSpace(*cmd.Git.SSHPrivateKey), string(authSecret.Data[util.PrivateKeySecretKey])) assert.Equal(t, authSecret.Annotations[util.ManagedByAnnotation], util.NctlName) @@ -289,6 +275,9 @@ func TestApplication(t *testing.T) { }, }, }, + checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, checkSecret: func(t *testing.T, cmd applicationCmd, authSecret *corev1.Secret) { assert.Equal(t, *cmd.Git.Username, string(authSecret.Data[util.UsernameSecretKey])) assert.Equal(t, *cmd.Git.Password, string(authSecret.Data[util.PasswordSecretKey])) @@ -322,6 +311,7 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.Equal(t, *cmd.Git.URL, updated.Spec.ForProvider.Git.URL) }, checkSecret: func(t *testing.T, cmd applicationCmd, authSecret *corev1.Secret) { @@ -330,7 +320,20 @@ func TestApplication(t *testing.T) { }, }, "disable deploy job": { - orig: existingApp, + orig: func() *apps.Application { + a := existingApp.DeepCopy() + a.Spec.ForProvider.Config.DeployJob = &apps.DeployJob{ + Job: apps.Job{ + Command: "date", + Name: "print-date", + }, + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(int32(2)), + Timeout: &metav1.Duration{Duration: time.Minute}, + }, + } + return a + }(), gitAuth: &util.GitAuth{ SSHPrivateKey: ptr.To("fakekey"), }, @@ -341,7 +344,9 @@ func TestApplication(t *testing.T) { Git: &gitConfig{ URL: ptr.To("https://newgit.example.org"), }, - DeployJob: &deployJob{Enabled: ptr.To(false)}, + baseConfig: baseConfig{ + DeployJob: &deployJob{Enabled: ptr.To(false)}, + }, }, gitInformationServiceResponse: test.GitInformationServiceResponse{ Code: 200, @@ -357,9 +362,33 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { - assert.Nil(t, updated.Spec.ForProvider.Config.DeployJob) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, + }, + "remove worker job": { + orig: func() *apps.Application { + a := existingApp.DeepCopy() + a.Spec.ForProvider.Config.WorkerJobs = []apps.WorkerJob{ + { + Job: apps.Job{Name: "do-0", Command: "echo 0"}, + Size: ptr.To(test.AppStandard1), + }, + } + return a + }(), + cmd: applicationCmd{ + resourceCmd: resourceCmd{ + Name: existingApp.Name, + }, + baseConfig: baseConfig{ + DeleteWorkerJob: ptr.To("do-0"), + }, + }, + checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, + // "remvoe scheduled job": {}, "retry release": { orig: existingApp, cmd: applicationCmd{ @@ -370,6 +399,11 @@ func TestApplication(t *testing.T) { }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { assert.NotNil(t, util.EnvVarByName(updated.Spec.ForProvider.Config.Env, ReleaseTrigger)) + + // strip out the ReleaseTrigger entry before comparing everything else: + normalizedUpdatedConfig := updated.Spec.ForProvider.Config.DeepCopy() + normalizedUpdatedConfig.Env = removeEnvVar(normalizedUpdatedConfig.Env, ReleaseTrigger) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, *normalizedUpdatedConfig) }, }, "do not retry release": { @@ -382,6 +416,7 @@ func TestApplication(t *testing.T) { }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { assert.Nil(t, util.EnvVarByName(updated.Spec.ForProvider.Config.Env, ReleaseTrigger)) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "retry build": { @@ -394,6 +429,7 @@ func TestApplication(t *testing.T) { }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { assert.NotNil(t, util.EnvVarByName(updated.Spec.ForProvider.BuildEnv, BuildTrigger)) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "do not retry build": { @@ -406,6 +442,7 @@ func TestApplication(t *testing.T) { }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { assert.Nil(t, util.EnvVarByName(updated.Spec.ForProvider.BuildEnv, BuildTrigger)) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "disabling the git repo check works": { @@ -426,6 +463,7 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.Equal(t, *cmd.Git.URL, updated.Spec.ForProvider.Git.URL) }, }, @@ -498,6 +536,7 @@ func TestApplication(t *testing.T) { }, }, checkApp: func(t *testing.T, cmd applicationCmd, orig, updated *apps.Application) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) assert.Equal(t, "https://github.com/ninech/new-repo", updated.Spec.ForProvider.Git.URL) assert.Equal(t, "main", updated.Spec.ForProvider.Git.Revision) }, @@ -551,6 +590,32 @@ func TestApplication(t *testing.T) { } } +func newBaseConfigCmdAllFields() baseConfig { + return baseConfig{ + Size: ptr.To("newsize"), + Port: ptr.To(int32(1000)), + Replicas: ptr.To(int32(2)), + Env: map[string]string{"zoo": "bar"}, + BasicAuth: ptr.To(true), + DeployJob: &deployJob{ + Command: ptr.To("exit 0"), Name: ptr.To("exit"), + Retries: ptr.To(int32(1)), Timeout: ptr.To(time.Minute * 5), + }, + WorkerJob: &workerJob{ + Name: ptr.To("do-stuff-1"), + Command: ptr.To("echo stuff1"), + Size: ptr.To(string(test.AppStandard1)), + }, + ScheduledJob: &scheduledJob{ + Command: ptr.To("sleep 5; date"), + Name: ptr.To("scheduled-1"), + Size: ptr.To(string(test.AppStandard1)), + Schedule: ptr.To("* * * * *"), + Retries: ptr.To(int32(2)), Timeout: ptr.To(time.Minute * 5), + }, + } +} + // TestApplicationFlags tests the behavior of kong's flag parser when using // pointers. As we rely on pointers to check if a user supplied a flag we also // want to test it in case this ever changes in future kong versions. @@ -573,3 +638,16 @@ func TestApplicationFlags(t *testing.T) { assert.NotNil(t, emptyFlags.Env) assert.NotNil(t, emptyFlags.BuildEnv) } + +// removeEnvVar drops any entry whose Name matches key, +// returning a new slice (nil if it ends up empty). +func removeEnvVar(env []apps.EnvVar, key string) []apps.EnvVar { + var out []apps.EnvVar + for _, e := range env { + if e.Name == key { + continue + } + out = append(out, e) + } + return out +} diff --git a/update/project_config.go b/update/project_config.go index 7edc1fbe..5deb23b6 100644 --- a/update/project_config.go +++ b/update/project_config.go @@ -7,19 +7,13 @@ import ( "github.com/crossplane/crossplane-runtime/pkg/resource" apps "github.com/ninech/apis/apps/v1alpha1" "github.com/ninech/nctl/api" - "github.com/ninech/nctl/api/util" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // all fields need to be pointers so we can detect if they have been set by // the user. type configCmd struct { - Size *string `help:"Size of the app."` - Port *int32 `help:"Port the app is listening on."` - Replicas *int32 `help:"Amount of replicas of the running app."` - Env map[string]string `help:"Environment variables which are passed to the app at runtime."` - BasicAuth *bool `help:"Enable/Disable basic authentication for applications."` - DeployJob *deployJob `embed:"" prefix:"deploy-job-"` + baseConfig } func (cmd *configCmd) Run(ctx context.Context, client *api.Client) error { @@ -45,22 +39,5 @@ func (cmd *configCmd) Run(ctx context.Context, client *api.Client) error { } func (cmd *configCmd) applyUpdates(cfg *apps.ProjectConfig) { - if cmd.Size != nil { - cfg.Spec.ForProvider.Config.Size = apps.ApplicationSize(*cmd.Size) - } - if cmd.Port != nil { - cfg.Spec.ForProvider.Config.Port = cmd.Port - } - if cmd.Replicas != nil { - cfg.Spec.ForProvider.Config.Replicas = cmd.Replicas - } - if cmd.Env != nil { - cfg.Spec.ForProvider.Config.Env = util.EnvVarsFromMap(cmd.Env) - } - if cmd.BasicAuth != nil { - cfg.Spec.ForProvider.Config.EnableBasicAuth = cmd.BasicAuth - } - if cmd.DeployJob != nil { - cmd.DeployJob.applyUpdates(&cfg.Spec.ForProvider.Config) - } + cmd.baseConfig.applyUpdates(&cfg.Spec.ForProvider.Config) } diff --git a/update/project_config_test.go b/update/project_config_test.go index b5a9ed12..de3c06a9 100644 --- a/update/project_config_test.go +++ b/update/project_config_test.go @@ -46,54 +46,184 @@ func TestConfig(t *testing.T) { "change port": { orig: existingConfig, cmd: configCmd{ - Port: ptr.To(int32(1234)), + baseConfig: baseConfig{ + Port: ptr.To(int32(1234)), + }, }, checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { - assert.Equal(t, *cmd.Port, *updated.Spec.ForProvider.Config.Port) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "port is unchanged when updating unrelated field": { orig: existingConfig, cmd: configCmd{ - Size: ptr.To("newsize"), + baseConfig: baseConfig{ + Size: ptr.To("newsize"), + }, }, checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { - assert.Equal(t, *orig.Spec.ForProvider.Config.Port, *updated.Spec.ForProvider.Config.Port) - assert.NotEqual(t, orig.Spec.ForProvider.Config.Size, updated.Spec.ForProvider.Config.Size) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "update basic auth": { orig: existingConfig, cmd: configCmd{ - BasicAuth: ptr.To(true), + baseConfig: baseConfig{ + BasicAuth: ptr.To(true), + }, }, checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { - assert.True(t, *updated.Spec.ForProvider.Config.EnableBasicAuth) + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) }, }, "all fields update": { orig: existingConfig, + cmd: configCmd{newBaseConfigCmdAllFields()}, + checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, + }, + "reset env variable": { + orig: existingConfig, + cmd: configCmd{ + baseConfig: baseConfig{ + DeleteEnv: &[]string{"foo"}, + }, + }, + checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, + }, + + "jobs union adds distinct element": { + orig: func() *apps.ProjectConfig { + c := existingConfig.DeepCopy() + c.Spec.ForProvider.Config.WorkerJobs = []apps.WorkerJob{ + { + Job: apps.Job{ + Name: "do-stuff-0", + Command: "echo stuff0", + }, + Size: ptr.To(test.AppStandard1), + }, + } + c.Spec.ForProvider.Config.ScheduledJobs = []apps.ScheduledJob{ + { + Job: apps.Job{ + Command: "sleep 1; date", + Name: "scheduled-0a", + }, + Size: ptr.To(test.AppMini), + Schedule: "* * * * *", + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(int32(2)), + Timeout: &metav1.Duration{Duration: time.Minute * 5}, + }, + }, + { + Job: apps.Job{ + Command: "sleep 2; date", + Name: "scheduled-0b", + }, + }, + } + return c + }(), + cmd: configCmd{ + baseConfig: baseConfig{ + Size: ptr.To("newsize"), + Port: ptr.To(int32(1000)), + Replicas: ptr.To(int32(2)), + Env: map[string]string{"zoo": "bar"}, + BasicAuth: ptr.To(true), + WorkerJob: &workerJob{ + Name: ptr.To("do-stuff-1"), + Command: ptr.To("echo stuff1"), + Size: ptr.To(string(test.AppStandard1)), + }, + ScheduledJob: &scheduledJob{ + Command: ptr.To("sleep 11; date"), + Name: ptr.To("scheduled-1"), + Size: ptr.To(string(test.AppStandard1)), + Schedule: ptr.To("1 * * * *"), + Retries: ptr.To(int32(2)), Timeout: ptr.To(time.Minute * 5), + }, + }, + }, + checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { + assertBaseConfig(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + }, + }, + "jobs union modifies existing element": { + orig: func() *apps.ProjectConfig { + c := existingConfig.DeepCopy() + c.Spec.ForProvider.Config.DeployJob = &apps.DeployJob{ + Job: apps.Job{Name: "init-0", Command: "sleep 1"}, + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(int32(1)), + Timeout: &metav1.Duration{Duration: time.Second * 10}, + }, + } + c.Spec.ForProvider.Config.WorkerJobs = []apps.WorkerJob{ + { + Job: apps.Job{Name: "work-0", Command: "sleep 2"}, + Size: ptr.To(test.AppStandard1), + }, + } + c.Spec.ForProvider.Config.ScheduledJobs = []apps.ScheduledJob{ + { + Job: apps.Job{Name: "sch-0", Command: "sleep 3"}, + Size: ptr.To(test.AppMini), + Schedule: "* * * * *", + FiniteJob: apps.FiniteJob{ + Retries: ptr.To(int32(2)), + Timeout: &metav1.Duration{Duration: time.Minute * 5}, + }, + }, + } + return c + }(), cmd: configCmd{ - Size: ptr.To("newsize"), - Port: ptr.To(int32(1000)), - Replicas: ptr.To(int32(2)), - Env: map[string]string{"zoo": "bar"}, - BasicAuth: ptr.To(true), - DeployJob: &deployJob{ - Command: ptr.To("exit 0"), Name: ptr.To("exit"), - Retries: ptr.To(int32(1)), Timeout: ptr.To(time.Minute * 5), + baseConfig: baseConfig{ + // note: we modify existing jobs here: + DeployJob: &deployJob{ + Name: ptr.To("init-0"), Command: ptr.To("echo 1"), + Timeout: ptr.To(time.Second * 3), + }, + WorkerJob: &workerJob{ + Name: ptr.To("work-0"), + Size: ptr.To(string(test.AppMicro)), + }, + ScheduledJob: &scheduledJob{ + Name: ptr.To("sch-0"), + Command: ptr.To("sleep 99"), + Schedule: ptr.To("3 * * * *"), + }, }, }, checkConfig: func(t *testing.T, cmd configCmd, orig, updated *apps.ProjectConfig) { - assert.Equal(t, apps.ApplicationSize(*cmd.Size), updated.Spec.ForProvider.Config.Size) - assert.Equal(t, *cmd.Port, *updated.Spec.ForProvider.Config.Port) - assert.Equal(t, *cmd.Replicas, *updated.Spec.ForProvider.Config.Replicas) - assert.Equal(t, *cmd.BasicAuth, *updated.Spec.ForProvider.Config.EnableBasicAuth) - assert.Equal(t, util.EnvVarsFromMap(cmd.Env), updated.Spec.ForProvider.Config.Env) - assert.Equal(t, *cmd.DeployJob.Command, updated.Spec.ForProvider.Config.DeployJob.Command) - assert.Equal(t, *cmd.DeployJob.Name, updated.Spec.ForProvider.Config.DeployJob.Name) - assert.Equal(t, *cmd.DeployJob.Timeout, updated.Spec.ForProvider.Config.DeployJob.Timeout.Duration) - assert.Equal(t, *cmd.DeployJob.Retries, *updated.Spec.ForProvider.Config.DeployJob.Retries) + // the basic configuration should not change, only the jobs are modified here: + assertBaseConfigCore(t, orig.Spec.ForProvider.Config, cmd.baseConfig, updated.Spec.ForProvider.Config) + + d := orig.Spec.ForProvider.Config.DeployJob.DeepCopy() + d.Command = "echo 1" + d.Timeout = &metav1.Duration{Duration: time.Second * 3} + wantDeployJobs := []apps.DeployJob{*d} + gotDeployJobs := test.PtrToSlice(updated.Spec.ForProvider.Config.DeployJob, func(d *apps.DeployJob) apps.DeployJob { return *d }) + test.AssertJobsEqual(t, wantDeployJobs, gotDeployJobs, func(d apps.DeployJob) test.DeployJobKey { return test.ToDeployJobKey(&d) }) + + w := orig.Spec.ForProvider.Config.WorkerJobs[0] + w.Size = ptr.To(test.AppMicro) + wantWorkerJobs := []apps.WorkerJob{w} + gotWorkerJobs := test.NormalizeSlice(updated.Spec.ForProvider.Config.WorkerJobs) + test.AssertJobsEqual(t, wantWorkerJobs, gotWorkerJobs, test.ToWorkerJobKey) + + j := orig.Spec.ForProvider.Config.ScheduledJobs[0] + j.Command = "sleep 99" + j.Schedule = "3 * * * *" + wantScheduledJobs := []apps.ScheduledJob{j} + gotScheduledJobs := test.NormalizeSlice(updated.Spec.ForProvider.Config.ScheduledJobs) + test.AssertJobsEqual(t, wantScheduledJobs, gotScheduledJobs, test.ToScheduledJobKey) }, }, } @@ -142,3 +272,193 @@ func TestProjectConfigFlags(t *testing.T) { assert.NotNil(t, emptyFlags.Env) } + +// assertBaseConfig verifies that apps.Config fields were correctly updated +// based on the provided CLI config. It checks both modified fields and ensures +// that unspecified fields remain unchanged. +func assertBaseConfig(t *testing.T, orig apps.Config, cmd baseConfig, got apps.Config) { + t.Helper() + + assertBaseConfigCore(t, orig, cmd, got) + assertBaseConfigJobs(t, orig, cmd, got) +} + +// assertBaseConfigCore asserts that the "core" flags (baseConfig) in CLI config +// ended up in the updated config, and that Env collection is updated correctly. +// It also ensures that unspecified fields remain unchanged. +func assertBaseConfigCore(t *testing.T, orig apps.Config, cmd baseConfig, got apps.Config) { + t.Helper() + + if cmd.Size != nil { + assert.Equal(t, apps.ApplicationSize(*cmd.Size), got.Size, "size") + } else { + assert.Equal(t, orig.Size, got.Size, "size (unchanged)") + } + if cmd.Port != nil { + assert.Equal(t, *cmd.Port, *got.Port, "port") + } else { + assert.Equal(t, *orig.Port, *got.Port, "port (unchanged)") + } + if cmd.Replicas != nil { + assert.Equal(t, *cmd.Replicas, *got.Replicas, "replicas") + } else { + assert.Equal(t, *orig.Replicas, *got.Replicas, "replicas (unchanged)") + } + if cmd.BasicAuth != nil { + assert.Equal(t, *cmd.BasicAuth, *got.EnableBasicAuth, "basic auth") + } else { + assert.Equal(t, *orig.EnableBasicAuth, *got.EnableBasicAuth, "basic auth (unchanged)") + } + if cmd.Env != nil || cmd.DeleteEnv != nil { + runtimeEnv := map[string]string{} + if cmd.Env != nil { + runtimeEnv = cmd.Env + } + var deleteEnv []string + if cmd.DeleteEnv != nil { + deleteEnv = *cmd.DeleteEnv + } + expectedEnv := util.UpdateEnvVars(orig.Env, runtimeEnv, deleteEnv) + assert.Equal(t, expectedEnv, got.Env, "env vars (added and/or deleted)") + } else { + assert.Equal(t, + orig.Env, got.Env, "env vars (unchanged)") + } +} + +// assertBaseConfigJobs provides a default logic for jobs assert. It checks +// both modified fields and ensures that unspecified fields remain unchanged. +func assertBaseConfigJobs(t *testing.T, orig apps.Config, cmd baseConfig, got apps.Config) { + t.Helper() + + var wantDeployJobs []apps.DeployJob + if cmd.DeployJob != nil { + if cmd.DeployJob.Enabled == nil || *cmd.DeployJob.Enabled { + wantDeployJobs = test.PtrToSlice(cmd.DeployJob, deployJobFromCmd) + } + } + gotDeployJobs := test.PtrToSlice(got.DeployJob, func(d *apps.DeployJob) apps.DeployJob { return *d }) + test.AssertJobsEqual(t, wantDeployJobs, gotDeployJobs, func(d apps.DeployJob) test.DeployJobKey { return test.ToDeployJobKey(&d) }) + + wantWorkerJobs := appendPtrSlice(orig.WorkerJobs, cmd.WorkerJob, workerJobFromCmd) + if cmd.DeleteWorkerJob != nil { + jobName := *cmd.DeleteWorkerJob + wantWorkerJobs = removeWorkerJob(wantWorkerJobs, jobName) + } + gotWorkerJobs := test.NormalizeSlice(got.WorkerJobs) + test.AssertJobsEqual(t, wantWorkerJobs, gotWorkerJobs, test.ToWorkerJobKey) + + wantScheduledJobs := appendPtrSlice(orig.ScheduledJobs, cmd.ScheduledJob, scheduledJobFromCmd) + if cmd.DeleteScheduledJob != nil { + jobName := *cmd.DeleteScheduledJob + wantScheduledJobs = removeScheduledJob(wantScheduledJobs, jobName) + } + gotScheduledJobs := test.NormalizeSlice(got.ScheduledJobs) + test.AssertJobsEqual(t, wantScheduledJobs, gotScheduledJobs, test.ToScheduledJobKey) +} + +// deployJobFromCmd converts the CLI deployJob into the CRD DeployJob. +func deployJobFromCmd(j *deployJob) apps.DeployJob { + if j == nil { + return apps.DeployJob{} + } + dj := apps.DeployJob{ + Job: apps.Job{ + Name: ptr.Deref(j.Name, ""), + Command: ptr.Deref(j.Command, ""), + }, + } + if j.Retries != nil { + dj.Retries = j.Retries + } + if j.Timeout != nil { + dj.Timeout = &metav1.Duration{Duration: *j.Timeout} + } + return dj +} + +// workerJobFromCmd converts the CLI workerJob into the CRD WorkerJob. +func workerJobFromCmd(j *workerJob) apps.WorkerJob { + if j == nil { + return apps.WorkerJob{} + } + return apps.WorkerJob{ + Job: apps.Job{ + Name: ptr.Deref(j.Name, ""), + Command: ptr.Deref(j.Command, ""), + }, + Size: ptr.To(apps.ApplicationSize(ptr.Deref(j.Size, ""))), + } +} + +// scheduledJobFromCmd turns the CLI representation into the form that finally +// ends up on the ProjectConfig after the mapping/conversion logic. +func scheduledJobFromCmd(j *scheduledJob) apps.ScheduledJob { + if j == nil { + return apps.ScheduledJob{} + } + + out := apps.ScheduledJob{ + Job: apps.Job{ + Name: ptr.Deref(j.Name, ""), + Command: ptr.Deref(j.Command, ""), + }, + Size: ptr.To(apps.ApplicationSize(ptr.Deref(j.Size, ""))), + Schedule: ptr.Deref(j.Schedule, ""), + } + if j.Retries != nil { + out.Retries = new(int32) + *out.Retries = *j.Retries + } + if j.Timeout != nil { + out.Timeout = new(metav1.Duration) + out.Timeout.Duration = *j.Timeout + } + return out +} + +// appendPtrSlice takes any existing slice of Ts plus an optional +// *S from the CLI and returns the append. +func appendPtrSlice[S any, T any](orig []T, cli *S, conv func(*S) T) []T { + // copy orig so we don't clobber it + out := append([]T(nil), orig...) + if cli != nil { + out = append(out, conv(cli)) + } + return out +} + +// removeJobByName returns a new slice containing only those items in orig +// whose name (as extracted by getName) does *not* equal the given name. +// If no items match, it simply returns a copy of orig. +func removeJobByName[T any]( + orig []T, + name string, + getName func(T) string, +) []T { + var out []T + for _, item := range orig { + if getName(item) != name { + out = append(out, item) + } + } + return out +} + +func removeWorkerJob( + orig []apps.WorkerJob, + name string, +) []apps.WorkerJob { + return removeJobByName(orig, name, func(w apps.WorkerJob) string { + return w.Job.Name + }) +} + +func removeScheduledJob( + orig []apps.ScheduledJob, + name string, +) []apps.ScheduledJob { + return removeJobByName(orig, name, func(s apps.ScheduledJob) string { + return s.Job.Name + }) +} From adae8a0589794d1bfa2a2382524f95ae537cd2d8 Mon Sep 17 00:00:00 2001 From: Pawel Kuc Date: Tue, 6 May 2025 19:41:29 +0200 Subject: [PATCH 2/3] feat: remove local job validation logic in favor of webhook-based CRD validation We now rely on the Kubernetes admission webhook deployed for our CRD backend, which performs full validation of the config, including DeployJob. The removed logic only checked the job name and did not cover other important fields, making it insufficient for robust validation. --- api/validation/config.go | 28 -------------------------- create/application.go | 9 --------- create/application_test.go | 40 -------------------------------------- update/application.go | 9 --------- 4 files changed, 86 deletions(-) delete mode 100644 api/validation/config.go diff --git a/api/validation/config.go b/api/validation/config.go deleted file mode 100644 index a6526a1e..00000000 --- a/api/validation/config.go +++ /dev/null @@ -1,28 +0,0 @@ -package validation - -import ( - "errors" - - apps "github.com/ninech/apis/apps/v1alpha1" -) - -// ConfigValidator validates a Config -type ConfigValidator struct { - Config apps.Config -} - -// TODO: remove local job validation logic in favor of webhook-based CRD validation -// We now rely on the Kubernetes admission webhook deployed for our CRD backend, -// which performs full validation of the config, including DeployJob. The removed -// logic only checked the job name and did not cover other important fields, -// making it insufficient for robust validation. - -// Validate validates the config -func (c ConfigValidator) Validate() error { - if c.Config.DeployJob != nil { - if len(c.Config.DeployJob.Name) == 0 { - return errors.New("deploy job name cannot be empty") - } - } - return nil -} diff --git a/create/application.go b/create/application.go index b988c5f5..db0ded4d 100644 --- a/create/application.go +++ b/create/application.go @@ -177,15 +177,6 @@ func (app *applicationCmd) Run(ctx context.Context, client *api.Client) error { } } - if newApp.Spec.ForProvider.Config.DeployJob != nil { - configValidator := &validation.ConfigValidator{ - Config: newApp.Spec.ForProvider.Config, - } - if err := configValidator.Validate(); err != nil { - return fmt.Errorf("error when validating application config: %w", err) - } - } - c := newCreator(client, newApp, strings.ToLower(apps.ApplicationKind)) appWaitCtx, cancel := context.WithTimeout(ctx, app.WaitTimeout) defer cancel() diff --git a/create/application_test.go b/create/application_test.go index afb96399..1f54445e 100644 --- a/create/application_test.go +++ b/create/application_test.go @@ -269,46 +269,6 @@ func TestApplication(t *testing.T) { }, errorExpected: true, }, - // TODO: remove local job validation logic in favor of webhook-based CRD validation - "deploy job empty command": { - cmd: applicationCmd{ - Git: gitConfig{ - URL: "https://github.com/ninech/doesnotexist.git", - }, - resourceCmd: resourceCmd{ - Wait: false, - Name: "deploy-job-empty-command", - }, - baseConfig: baseConfig{ - Size: ptr.To("mini"), - DeployJob: deployJob{Command: "", Name: "print-date", Retries: 2, Timeout: time.Minute}, - }, - SkipRepoAccessCheck: true, - }, - checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { - assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) - }, - }, - // TODO: remove local job validation logic in favor of webhook-based CRD validation - "deploy job empty name": { - cmd: applicationCmd{ - Git: gitConfig{ - URL: "https://github.com/ninech/doesnotexist.git", - }, - resourceCmd: resourceCmd{ - Wait: false, - Name: "deploy-job-empty-name", - }, - baseConfig: baseConfig{ - Size: ptr.To("mini"), - DeployJob: deployJob{Command: "date", Name: "", Retries: 2, Timeout: time.Minute}, - }, - SkipRepoAccessCheck: true, - }, - checkApp: func(t *testing.T, cmd applicationCmd, app *apps.Application) { - assertBaseConfig(t, cmd.baseConfig, app.Spec.ForProvider.Config) - }, - }, "git-information-service happy path": { cmd: applicationCmd{ Git: gitConfig{ diff --git a/update/application.go b/update/application.go index 08d9064b..197c4e0e 100644 --- a/update/application.go +++ b/update/application.go @@ -202,15 +202,6 @@ func (cmd *applicationCmd) Run(ctx context.Context, client *api.Client) error { } } - if app.Spec.ForProvider.Config.DeployJob != nil { - configValidator := &validation.ConfigValidator{ - Config: app.Spec.ForProvider.Config, - } - if err := configValidator.Validate(); err != nil { - return fmt.Errorf("error when validating application config: %w", err) - } - } - return nil }) From 3f3c3f14e235612c468854bd4c612ef4fa775a6f Mon Sep 17 00:00:00 2001 From: Pawel Kuc Date: Tue, 6 May 2025 21:15:18 +0200 Subject: [PATCH 3/3] fix: inject ApplicationKongVars in TestProjectConfigFlags to define defaults Add a call to ApplicationKongVars in TestProjectConfigFlags so that all app_default_* variables (e.g. ${app_default_size} in the help/placeholder strings) are populated before invoking kong.Must(). This change prevents build-time panics from unresolved placeholders. --- update/project_config_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/update/project_config_test.go b/update/project_config_test.go index de3c06a9..818705c0 100644 --- a/update/project_config_test.go +++ b/update/project_config_test.go @@ -8,6 +8,7 @@ import ( "github.com/alecthomas/kong" apps "github.com/ninech/apis/apps/v1alpha1" "github.com/ninech/nctl/api/util" + "github.com/ninech/nctl/create" "github.com/ninech/nctl/internal/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -261,13 +262,15 @@ func TestConfig(t *testing.T) { // want to test it in case this ever changes in future kong versions. func TestProjectConfigFlags(t *testing.T) { nilFlags := &configCmd{} - _, err := kong.Must(nilFlags).Parse([]string{}) + vars, err := create.ApplicationKongVars() + require.NoError(t, err) + _, err = kong.Must(nilFlags, vars).Parse([]string{}) require.NoError(t, err) assert.Nil(t, nilFlags.Env) emptyFlags := &configCmd{} - _, err = kong.Must(emptyFlags).Parse([]string{`--env=`}) + _, err = kong.Must(emptyFlags, vars).Parse([]string{`--env=`}) require.NoError(t, err) assert.NotNil(t, emptyFlags.Env)