-
Notifications
You must be signed in to change notification settings - Fork 25
feat(routing-table): provide rt and routes functionality #1023
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
5b9fe56 to
45f6702
Compare
| cmd.Flags().Var(flags.CIDRFlag(), destinationValueFlag, "Destination value") | ||
| cmd.Flags().String(nextHopValueFlag, "", "NextHop value") | ||
|
|
||
| cmd.Flags().Var( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced an enum flag here. I'm not certain if this matches our intended cli design, but I felt it would be beneficial to validate values early.
| Labels: flags.FlagToStringToStringPointer(p, cmd, labelFlag), | ||
| } | ||
|
|
||
| // Next Hop validation logic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here I felt like it would be beneficial to validate values early. Can be tested with commands like:
bin/stackit beta routing-table route create --destination-type cidrv4 --destination-value 0.0.0.0/32 --nexthop-type internet --nexthop-value 1.1.1.1 --network-area-id xxx --organization-id yyy --routing-table-id zzz -o pretty
Expected: Error: --nexthop-value is not allowed when --nexthop-type is 'internet' or 'blackhole'
| Short: "Manage routing-tables and its according routes", | ||
| Long: `Manage routing tables and their associated routes. | ||
|
|
||
| This functionality is currently in BETA. At this stage, only listing and describing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this to give the user a hint, that this is more used for debugging rather than being used for creating routing-tables and attaching them to networks.
|
Update waiting for go-sdk iaas api update ^ |
| "github.com/stackitcloud/stackit-sdk-go/services/iaasalpha" | ||
| ) | ||
|
|
||
| var testRegion = "eu01" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be const
| var testOrgId = uuid.NewString() | ||
| var testNetworkAreaId = uuid.NewString() | ||
|
|
||
| var testLabelSelectorFlag = "key1=value1,key2=value2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be const
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
https://github.com/stackitcloud/stackit-cli/blob/main/internal/cmd/image/list/list_test.go#L129
see also the PR #1033 for reference
| var testCtx = context.WithValue(context.Background(), testCtxKey{}, "foo") | ||
| var testClient = &iaasalpha.APIClient{} | ||
|
|
||
| var testRegion = "eu01" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be const
| var testNetworkAreaId = uuid.NewString() | ||
| var testRoutingTableId = uuid.NewString() | ||
|
|
||
| var testDestinationTypeFlag = "cidrv4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
most variables here could be const
| } | ||
|
|
||
| func outputResult(p *print.Printer, outputFormat string, routingTable *iaasalpha.Route) error { | ||
| switch outputFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new Printer.OutputResult func here to save us the boilerplate code for the JSON/YAML output.
stackit-cli/internal/pkg/print/print.go
Lines 247 to 268 in 4725975
| func (p *Printer) OutputResult(outputFormat string, output any, prettyOutputFunc func() error) error { | |
| switch outputFormat { | |
| case JSONOutputFormat: | |
| details, err := json.MarshalIndent(output, "", " ") | |
| if err != nil { | |
| return fmt.Errorf("marshal json: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| case YAMLOutputFormat: | |
| details, err := yaml.MarshalWithOptions(output, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) | |
| if err != nil { | |
| return fmt.Errorf("marshal yaml: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| default: | |
| return prettyOutputFunc() | |
| } | |
| } |
stackit-cli/internal/cmd/image/list/list.go
Lines 139 to 178 in 4725975
| return p.OutputResult(outputFormat, items, func() error { | |
| table := tables.NewTable() | |
| table.SetHeader("ID", "NAME", "OS", "ARCHITECTURE", "DISTRIBUTION", "VERSION", "LABELS") | |
| for i := range items { | |
| item := items[i] | |
| var ( | |
| architecture string = "n/a" | |
| os string = "n/a" | |
| distro string = "n/a" | |
| version string = "n/a" | |
| ) | |
| if cfg := item.Config; cfg != nil { | |
| if v := cfg.Architecture; v != nil { | |
| architecture = *v | |
| } | |
| if v := cfg.OperatingSystem; v != nil { | |
| os = *v | |
| } | |
| if v := cfg.OperatingSystemDistro; v != nil && v.IsSet() { | |
| distro = *v.Get() | |
| } | |
| if v := cfg.OperatingSystemVersion; v != nil && v.IsSet() { | |
| version = *v.Get() | |
| } | |
| } | |
| table.AddRow(utils.PtrString(item.Id), | |
| utils.PtrString(item.Name), | |
| os, | |
| architecture, | |
| distro, | |
| version, | |
| utils.JoinStringKeysPtr(*item.Labels, ",")) | |
| } | |
| err := table.Display(p) | |
| if err != nil { | |
| return fmt.Errorf("render table: %w", err) | |
| } | |
| return nil | |
| }) |
see also the PR #1030 for reference
| } | ||
|
|
||
| if items := response.Items; items == nil || len(*items) == 0 { | ||
| params.Printer.Info("No routes found for routing-table %q\n", *model.RoutingTableId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid output in case the --output-format flag is set to JSON/YAML output
| } | ||
|
|
||
| // Truncate output | ||
| items := *response.Items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential nil pointer dereference (I guess, didn't take a closer look). Please use response.GetItems() or this util func
stackit-cli/internal/pkg/utils/utils.go
Lines 256 to 261 in 4725975
| func GetSliceFromPointer[T any](s *[]T) []T { | |
| if s == nil || *s == nil { | |
| return []T{} | |
| } | |
| return *s | |
| } |
| } | ||
|
|
||
| func outputResult(p *print.Printer, outputFormat string, items []iaasalpha.Route) error { | ||
| switch outputFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new Printer.OutputResult func here to save us the boilerplate code for the JSON/YAML output.
stackit-cli/internal/pkg/print/print.go
Lines 247 to 268 in 4725975
| func (p *Printer) OutputResult(outputFormat string, output any, prettyOutputFunc func() error) error { | |
| switch outputFormat { | |
| case JSONOutputFormat: | |
| details, err := json.MarshalIndent(output, "", " ") | |
| if err != nil { | |
| return fmt.Errorf("marshal json: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| case YAMLOutputFormat: | |
| details, err := yaml.MarshalWithOptions(output, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) | |
| if err != nil { | |
| return fmt.Errorf("marshal yaml: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| default: | |
| return prettyOutputFunc() | |
| } | |
| } |
stackit-cli/internal/cmd/image/list/list.go
Lines 139 to 178 in 4725975
| return p.OutputResult(outputFormat, items, func() error { | |
| table := tables.NewTable() | |
| table.SetHeader("ID", "NAME", "OS", "ARCHITECTURE", "DISTRIBUTION", "VERSION", "LABELS") | |
| for i := range items { | |
| item := items[i] | |
| var ( | |
| architecture string = "n/a" | |
| os string = "n/a" | |
| distro string = "n/a" | |
| version string = "n/a" | |
| ) | |
| if cfg := item.Config; cfg != nil { | |
| if v := cfg.Architecture; v != nil { | |
| architecture = *v | |
| } | |
| if v := cfg.OperatingSystem; v != nil { | |
| os = *v | |
| } | |
| if v := cfg.OperatingSystemDistro; v != nil && v.IsSet() { | |
| distro = *v.Get() | |
| } | |
| if v := cfg.OperatingSystemVersion; v != nil && v.IsSet() { | |
| version = *v.Get() | |
| } | |
| } | |
| table.AddRow(utils.PtrString(item.Id), | |
| utils.PtrString(item.Name), | |
| os, | |
| architecture, | |
| distro, | |
| version, | |
| utils.JoinStringKeysPtr(*item.Labels, ",")) | |
| } | |
| err := table.Display(p) | |
| if err != nil { | |
| return fmt.Errorf("render table: %w", err) | |
| } | |
| return nil | |
| }) |
see also the PR #1030 for reference
| } | ||
|
|
||
| func outputResult(p *print.Printer, outputFormat, routingTableId, networkAreaId string, route iaasalpha.Route) error { | ||
| switch outputFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new Printer.OutputResult func here to save us the boilerplate code for the JSON/YAML output.
stackit-cli/internal/pkg/print/print.go
Lines 247 to 268 in 4725975
| func (p *Printer) OutputResult(outputFormat string, output any, prettyOutputFunc func() error) error { | |
| switch outputFormat { | |
| case JSONOutputFormat: | |
| details, err := json.MarshalIndent(output, "", " ") | |
| if err != nil { | |
| return fmt.Errorf("marshal json: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| case YAMLOutputFormat: | |
| details, err := yaml.MarshalWithOptions(output, yaml.IndentSequence(true), yaml.UseJSONMarshaler()) | |
| if err != nil { | |
| return fmt.Errorf("marshal yaml: %w", err) | |
| } | |
| p.Outputln(string(details)) | |
| return nil | |
| default: | |
| return prettyOutputFunc() | |
| } | |
| } |
stackit-cli/internal/cmd/image/list/list.go
Lines 139 to 178 in 4725975
| return p.OutputResult(outputFormat, items, func() error { | |
| table := tables.NewTable() | |
| table.SetHeader("ID", "NAME", "OS", "ARCHITECTURE", "DISTRIBUTION", "VERSION", "LABELS") | |
| for i := range items { | |
| item := items[i] | |
| var ( | |
| architecture string = "n/a" | |
| os string = "n/a" | |
| distro string = "n/a" | |
| version string = "n/a" | |
| ) | |
| if cfg := item.Config; cfg != nil { | |
| if v := cfg.Architecture; v != nil { | |
| architecture = *v | |
| } | |
| if v := cfg.OperatingSystem; v != nil { | |
| os = *v | |
| } | |
| if v := cfg.OperatingSystemDistro; v != nil && v.IsSet() { | |
| distro = *v.Get() | |
| } | |
| if v := cfg.OperatingSystemVersion; v != nil && v.IsSet() { | |
| version = *v.Get() | |
| } | |
| } | |
| table.AddRow(utils.PtrString(item.Id), | |
| utils.PtrString(item.Name), | |
| os, | |
| architecture, | |
| distro, | |
| version, | |
| utils.JoinStringKeysPtr(*item.Labels, ",")) | |
| } | |
| err := table.Display(p) | |
| if err != nil { | |
| return fmt.Errorf("render table: %w", err) | |
| } | |
| return nil | |
| }) |
see also the PR #1030 for reference
Description
This PR adds functionality to list and describe routing tables, as well as perform full CRUD operations on routes within those tables. It does not include support for creating or attaching routing tables, as those operations are currently intended to be managed exclusively through Terraform.
This implementation is primarily aimed at enabling users to inspect and debug routes created via Terraform. Once the routing table feature reaches GA, support for creating routing tables and attaching them to networks will be added to the CLI.
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)