Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reject invalid endpoint #320

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Reject invalid endpoint #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f683c2a
Reject invalid endpoint
rmweir Sep 3, 2024
40b2102
Update sqlite test
rmweir Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 27 additions & 7 deletions pkg/endpoint/endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ type ETCDConfig struct {
}

func Listen(ctx context.Context, config Config) (ETCDConfig, error) {
driver, dsn := ParseStorageEndpoint(config.Endpoint)
driver, dsn, err := ParseStorageEndpoint(config.Endpoint)
if err != nil {
return ETCDConfig{}, errors.Wrap(err, "parsing storage endpoint")
}

if driver == ETCDBackend {
return ETCDConfig{
Endpoints: strings.Split(config.Endpoint, ","),
Expand Down Expand Up @@ -233,19 +237,35 @@ func getKineStorageBackend(ctx context.Context, driver, dsn string, cfg Config)
}

// ParseStorageEndpoint returns the driver name and endpoint string from a datastore endpoint URL.
func ParseStorageEndpoint(storageEndpoint string) (string, string) {
func ParseStorageEndpoint(storageEndpoint string) (string, string, error) {
if storageEndpoint == "" {
return SQLiteBackend, "", nil
}

if err := validateDSNuri(storageEndpoint); err != nil {
return "", "", err
}

network, address := networkAndAddress(storageEndpoint)

switch network {
case "":
return SQLiteBackend, ""
case "nats":
return NATSBackend, storageEndpoint
return NATSBackend, storageEndpoint, nil
case "http":
fallthrough
case "https":
return ETCDBackend, address
return ETCDBackend, address, nil
}
return network, address, nil
}

// validateDSNuri ensure that the given string is of that format <scheme://<authority>
func validateDSNuri(str string) error {
parts := strings.SplitN(str, "://", 2)
if len(parts) > 1 {
return nil
}
return network, address
return errors.New("invalid datastore endpoint; endpoint should be a DSN URI in the format <scheme>://<authority>")
}

// networkAndAddress crudely splits a URL string into network (scheme) and address,
Expand Down
2 changes: 1 addition & 1 deletion scripts/test-helpers
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ provision-kine() {
$KINE_ENV \
$KINE_IMAGE \
--watch-progress-notify-interval=5s \
--endpoint $KINE_ENDPOINT
--endpoint=$KINE_ENDPOINT

local ip=$(docker container inspect --format '{{.NetworkSettings.IPAddress}}' $name | tee $TEST_DIR/kine/$count/metadata/ip)
local port=$(docker container inspect --format '{{range $k, $v := .NetworkSettings.Ports}}{{printf "%s\n" $k}}{{end}}' $name | head -n 1 | cut -d/ -f1 | tee $TEST_DIR/kine/$count/metadata/port)
Expand Down
2 changes: 1 addition & 1 deletion scripts/test-run-sqlite
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/bin/bash
start-test() {
KINE_IMAGE=$IMAGE KINE_ENDPOINT="sqlite" provision-kine
KINE_IMAGE=$IMAGE KINE_ENDPOINT="" provision-kine
Copy link
Member

Choose a reason for hiding this comment

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

[sqlite] time="2024-09-03T21:27:16Z" level=fatal msg="flag needs an argument: -endpoint"

You can leave --endpoint unset, but you can't currently set it to an empty string... so you should either fix the CLI flag, or explicitly set it.

Suggested change
KINE_IMAGE=$IMAGE KINE_ENDPOINT="" provision-kine
KINE_IMAGE=$IMAGE KINE_ENDPOINT="sqlite://" provision-kine

Copy link
Member

@brandond brandond Sep 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually I suspect that when the var is empty the script is just running kine --endpoint instead of kine --endpoint "" so the CLI framework is complaining that the flag doesn't have a value - which is valid.

You could change this

kine/scripts/test-helpers

Line 254 in cb8c874

--endpoint $KINE_ENDPOINT

to --endpoint=$KINE_ENDPOINT

local kine_url=$(cat $TEST_DIR/kine/*/metadata/url)
K3S_DATASTORE_ENDPOINT=$kine_url provision-cluster
}
Expand Down