Skip to content

feat(sca): onboard service command - #1632

Open
JYisus wants to merge 8 commits into
stackitcloud:mainfrom
JYisus:feat/list-applications
Open

JYisus wants to merge 8 commits into
stackitcloud:mainfrom
JYisus:feat/list-applications

Conversation

@JYisus

@JYisus JYisus commented Sep 29, 2026

Copy link
Copy Markdown

Description

relates to #1234

Checklist

  • Issue was linked above
  • Code format was applied: make fmt
  • Examples were added / adjusted (see e.g. here)
  • Docs are up-to-date: make generate-docs (will be checked by CI)
  • Unit tests got implemented or updated
  • Unit tests are passing: make test (will be checked by CI)
  • No linter issues: make lint (will be checked by CI)

@JYisus
JYisus requested a review from a team as a code owner September 29, 2026 08:27

@SerseusWasTaken SerseusWasTaken left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please also move the implementation to a alpha package, similar like the beta package exists, since the API is still in alpha

Long: "Create a STACKIT Kubernetes Engine (SCA) application from payload.",
Args: args.NoArgs,
Example: examples.Build(
// TODO: fix examples

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open todo

}
}

payload.DisplayName = flags.FlagToStringValue(p, cmd, nameFlag)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

causes nil pointer dereference when no paylaod is set

Comment on lines +129 to +137
payloadValue := flags.FlagToStringPointer(p, cmd, payloadFlag)
var payload *sca.CreateApplicationPayload
if payloadValue != nil {
payload = &sca.CreateApplicationPayload{}
err := json.Unmarshal([]byte(*payloadValue), payload)
if err != nil {
return nil, fmt.Errorf("enconde payload: %w", err)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs state default payload is used when none is set - this is not the case

}
}

payload.AdditionalProperties = nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential nil pointer dereference when no payload is set

Comment on lines +22 to +49
const (
environmentIDFlag = "environment-id"
nameFlag = "name"
imageFlag = "image"
publicFlag = "public"
externalPortFlag = "external-port"
cpuFlag = "cpu"
memoryFlag = "memory"
instancesFlag = "instances"
minInstancesFlag = "min-instances"
maxInstancesFlag = "max-instances"
scaleToZeroFlag = "scale-to-zero"
envVarsFlag = "environment-vars"
commandsFlag = "commands"
argsFlag = "args"
)

const (
scalingTypeManual = "manual"
scalingTypeAuto = "auto"

defaultPublic = true
defaultPort = 8080
defaultCPU = 1000
defaultMemory = 1024
defaultInstances = 1
defaultContainerName = "container-1"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: We can use one const () block for this

@@ -0,0 +1,199 @@
package update

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file has no tests

Long: "Update a STACKIT Kubernetes Engine (SCA) application.",
Args: args.SingleArg(applicationIDArg, nil),
Example: examples.Build(
// TODO: fix examples

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open todo

Comment on lines +175 to +176
Concurrency: &model.Concurrency,
Rps: &model.RPS,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like concurrency and RPS flags are not wired up - this buildScalingRules function always returns nil

Comment on lines +242 to +243
cmd.Flags().Int32(minInstancesFlag, 0, "The minimum number of application instances (if autoscaling is enabled)")
cmd.Flags().Int32(maxInstancesFlag, 0, "The maximum number of application instances (if autoscaling is enabled)")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is 0 for both flags a sensible default value?

Comment on lines +107 to +121
func parseInput(p *print.Printer, cmd *cobra.Command, _ []string) (*inputModel, error) {
globalFlags := globalflags.Parse(p, cmd)
if globalFlags.ProjectId == "" {
return nil, &errors.ProjectIdError{}
}

model := inputModel{
GlobalFlagModel: globalFlags,
EnvironmentID: flags.FlagToStringValue(p, cmd, environmentIDFlag),
Limit: flags.FlagToInt64Pointer(p, cmd, limitFlag),
}

p.DebugInputModel(model)
return &model, nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing validation for limit flag

@SerseusWasTaken SerseusWasTaken added the needs-work PR needs work from author. label Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain:Serverless needs-work PR needs work from author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants