Skip to content

Commit

Permalink
refactor: Improve UX when using port print format string (#1359)
Browse files Browse the repository at this point in the history
## Description:
<!-- Describe this change, how it works, and the motivation behind it.
-->
This PR introduces:
- Argument order validation check
- Custom formatting when only one value is specified on format string
- Unit tests for formatting functions
- CI test for the CLI command

This PR also fixes the default flag of the command

## Is this change user facing?
YES
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
Closes #1355 
Closes #1356  
Closes #1357
  • Loading branch information
victorcolombo committed Sep 21, 2023
1 parent f092bf5 commit 269fe88
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 6 deletions.
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,9 @@ jobs:
# Execute github starlark module
- run: "${KURTOSIS_BINPATH} run --enclave test-datastore github.com/kurtosis-tech/datastore-army-package '{\"num_datastores\": 2}'"

# Execute port print
- run: "${KURTOSIS_BINPATH} port print test-datastore datastore-0 grpc"

# Execute docker compose import
- run: "${KURTOSIS_BINPATH} import --enclave test-import --env << pipeline.parameters.dockerimport-cli-dotenv-relative-path >> << pipeline.parameters.dockerimport-cli-dockerfile-relative-path >>"

Expand Down
41 changes: 35 additions & 6 deletions cli/cli/commands/port/print/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,14 @@ const (
ipAddress = "127.0.0.1"
)

var expectedRelativeOrder = map[string]int{
protocolStr: 0,
ipStr: 1,
numberStr: 2, //nolint:gomnd
}

var (
formatFlagKeyDefault = fmt.Sprintf("'%s,%s,%s'", protocolStr, ipStr, numberStr)
formatFlagKeyDefault = fmt.Sprintf("%s,%s,%s", protocolStr, ipStr, numberStr)
formatFlagKeyExamples = []string{
fmt.Sprintf("'%s'", ipStr),
fmt.Sprintf("'%s'", numberStr),
Expand All @@ -63,7 +69,7 @@ var PortPrintCmd = &engine_consuming_kurtosis_command.EngineConsumingKurtosisCom
{
Key: formatFlagKey,
Usage: fmt.Sprintf(
"Allows selecting what pieces of port are printed, using comma separated values (examples: %s). Default %s.",
"Allows selecting what pieces of port are printed, using comma separated values (examples: %s). Default '%s'.",
strings.Join(formatFlagKeyExamples, ", "), formatFlagKeyDefault),
Type: flags.FlagType_String,
Default: formatFlagKeyDefault,
Expand Down Expand Up @@ -144,22 +150,29 @@ func run(
)
}

fullUrl, err := formatOutput(format, ipAddress, publicPort)
fullUrl, err := formatPortOutput(format, ipAddress, publicPort)
if err != nil {
return stacktrace.Propagate(err, "Couldn't format the output according to formatting string '%v'", format)
}
out.PrintOutLn(fullUrl)
return nil
}

func formatOutput(format string, ipAddress string, spec *services.PortSpec) (string, error) {
func formatPortOutput(format string, ipAddress string, spec *services.PortSpec) (string, error) {
parts := strings.Split(format, ",")
var resultParts []string
isOnlyPiece := len(parts) == 1
lastPart := parts[0]
for _, part := range parts {
if partIndex := expectedRelativeOrder[part]; partIndex < expectedRelativeOrder[lastPart] {
return "", stacktrace.NewError("Found '%v' before '%v', which is not expected.", lastPart, part)
}
}
for _, part := range parts {
switch part {
case protocolStr:
if spec.GetMaybeApplicationProtocol() != "" {
resultParts = append(resultParts, spec.GetMaybeApplicationProtocol()+"://")
resultParts = append(resultParts, getApplicationProtocol(spec.GetMaybeApplicationProtocol(), isOnlyPiece))
} else {
// TODO(victor.colombo): What should we do here? Panic? Warn?
// Left it as a debug for now so it doesn't pollute the output
Expand All @@ -168,10 +181,26 @@ func formatOutput(format string, ipAddress string, spec *services.PortSpec) (str
case ipStr:
resultParts = append(resultParts, ipAddress)
case numberStr:
resultParts = append(resultParts, fmt.Sprintf(":%d", spec.GetNumber()))
resultParts = append(resultParts, getPortString(spec.GetNumber(), isOnlyPiece))
default:
return "", stacktrace.NewError("Invalid format piece '%v'", part)
}
}
return strings.Join(resultParts, ""), nil
}

func getPortString(portNumber uint16, isOnlyPiece bool) string {
if isOnlyPiece {
return fmt.Sprintf("%d", portNumber)
} else {
return fmt.Sprintf(":%d", portNumber)
}
}

func getApplicationProtocol(applicationProtocol string, isOnlyPiece bool) string {
if isOnlyPiece {
return applicationProtocol
} else {
return fmt.Sprintf("%s://", applicationProtocol)
}
}
41 changes: 41 additions & 0 deletions cli/cli/commands/port/print/print_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package print

import (
"fmt"
"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/services"
"github.com/stretchr/testify/assert"
"testing"
)

const (
testIpAddress = "127.0.0.1"
port = 123
applicationProtocol = "http"
transportProtocol = services.TransportProtocol_TCP
)

var portSpec = services.NewPortSpec(port, transportProtocol, applicationProtocol)

func TestOnlyPiece(t *testing.T) {
inputExpectedOutputMap := map[string]string{
"number": fmt.Sprintf("%d", port),
"protocol": applicationProtocol,
"ip": testIpAddress,
}
for in, expected := range inputExpectedOutputMap {
out, err := formatPortOutput(in, testIpAddress, portSpec)
assert.NoError(t, err)
assert.Equal(t, expected, out)
}
}

func TestAllPieces(t *testing.T) {
out, err := formatPortOutput("protocol,ip,number", testIpAddress, portSpec)
assert.NoError(t, err)
assert.Equal(t, "http://127.0.0.1:123", out)
}

func TestPiecesOutOfOrder(t *testing.T) {
_, err := formatPortOutput("ip,protocol", testIpAddress, portSpec)
assert.Error(t, err)
}

0 comments on commit 269fe88

Please sign in to comment.