From da2da65cb27b83cdeb8ce89c609fe87bcb5c08c8 Mon Sep 17 00:00:00 2001 From: 6za <53096417+6za@users.noreply.github.com> Date: Thu, 13 Oct 2022 18:00:44 +0000 Subject: [PATCH 1/5] fix #514 Signed-off-by: 6za <53096417+6za@users.noreply.github.com> --- cmd/createGitlab.go | 97 +++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/cmd/createGitlab.go b/cmd/createGitlab.go index 072f93aa5..2d5de7099 100644 --- a/cmd/createGitlab.go +++ b/cmd/createGitlab.go @@ -3,6 +3,12 @@ package cmd import ( "crypto/tls" "fmt" + "log" + "net/http" + "os/exec" + "syscall" + "time" + "github.com/kubefirst/kubefirst/configs" "github.com/kubefirst/kubefirst/internal/argocd" "github.com/kubefirst/kubefirst/internal/flagset" @@ -16,12 +22,6 @@ import ( "github.com/kubefirst/kubefirst/pkg" "github.com/spf13/cobra" "github.com/spf13/viper" - "log" - "net/http" - "os" - "os/exec" - "syscall" - "time" ) // createGitlabCmd represents the createGitlab command @@ -372,42 +372,61 @@ var createGitlabCmd = &cobra.Command{ log.Println(err) } } - - kPortForwardVault, err = k8s.PortForward(globalFlags.DryRun, "vault", "svc/vault", "8200:8200") - defer func(Process *os.Process, sig os.Signal) { - err := Process.Signal(sig) - if err != nil { - log.Println(err) + // enable GitLab port forward connection for Terraform + if !globalFlags.DryRun { + // force close port forward, GitLab port forward is not available at this point anymore + //Close PF before creating new ones in the loop + if kPortForwardGitlab != nil { + err = kPortForwardGitlab.Process.Signal(syscall.SIGTERM) + if err != nil { + log.Println(err) + } } - }(kPortForwardVault.Process, syscall.SIGTERM) - - // force close port forward, GitLab port forward is not available at this point anymore - if kPortForwardGitlab != nil { - err = kPortForwardGitlab.Process.Signal(syscall.SIGTERM) - if err != nil { - log.Println(err) + if kPortForwardVault != nil { + err = kPortForwardVault.Process.Signal(syscall.SIGTERM) + if err != nil { + log.Println(err) + } } - } - - kPortForwardGitlab, err = k8s.PortForward(globalFlags.DryRun, "gitlab", "svc/gitlab-webservice-default", "8888:8080") - defer func(Process *os.Process, sig os.Signal) { - err := Process.Signal(sig) - if err != nil { - log.Println(err) + if !viper.GetBool("create.terraformapplied.users") { + for i := 0; i < totalAttempts; i++ { + kPortForwardVault, err = k8s.PortForward(globalFlags.DryRun, "vault", "svc/vault", "8200:8200") + defer func() { + _ = kPortForwardVault.Process.Signal(syscall.SIGTERM) + }() + kPortForwardGitlab, err = k8s.PortForward(globalFlags.DryRun, "gitlab", "svc/gitlab-webservice-default", "8888:8080") + defer func() { + _ = kPortForwardGitlab.Process.Signal(syscall.SIGTERM) + }() + if err != nil { + log.Println("Error creating port-forward") + return err + } + //Wait the universe to sort itself out before creating more changes + time.Sleep(20 * time.Second) + + // manage users via Terraform + directory = fmt.Sprintf("%s/gitops/terraform/users", config.K1FolderPath) + informUser("applying users terraform", globalFlags.SilentMode) + gitProvider := viper.GetString("git.mode") + err = terraform.ApplyUsersTerraform(globalFlags.DryRun, directory, gitProvider) + if err != nil { + log.Println("Error applying users") + log.Println(err) + } + if viper.GetBool("create.terraformapplied.users") || err == nil { + log.Println("Users configured") + break + } else { + log.Println("Users not configured - waiting before trying again") + time.Sleep(20 * time.Second) + _ = kPortForwardGitlab.Process.Signal(syscall.SIGTERM) + _ = kPortForwardVault.Process.Signal(syscall.SIGTERM) + } + } + } else { + log.Println("Skipped - Users configured") } - }(kPortForwardGitlab.Process, syscall.SIGTERM) - if err != nil { - log.Println("Error creating port-forward") - return err - } - - // manage users via Terraform - directory = fmt.Sprintf("%s/gitops/terraform/users", config.K1FolderPath) - informUser("applying users terraform", globalFlags.SilentMode) - gitProvider := viper.GetString("git.mode") - err = terraform.ApplyUsersTerraform(globalFlags.DryRun, directory, gitProvider) - if err != nil { - return err } return nil From 2d2d29295d451f7598816c699543245ff15f59e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vanzuita?= Date: Thu, 13 Oct 2022 15:05:04 -0300 Subject: [PATCH 2/5] feat: filter subdomains in telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Vanzuita --- cmd/create.go | 21 +++++--- cmd/init.go | 22 ++++++--- internal/domain/telemetry.go | 37 ++++++++++++++ internal/domain/telemetry_test.go | 76 +++++++++++++++++++++++++++++ internal/handlers/telemetry.go | 25 +++++----- internal/handlers/telemetry_test.go | 63 +++++++++--------------- pkg/helpers.go | 37 ++++++++++++++ pkg/helpers_test.go | 74 ++++++++++++++++++++++++++++ 8 files changed, 285 insertions(+), 70 deletions(-) create mode 100644 internal/domain/telemetry.go create mode 100644 internal/domain/telemetry_test.go create mode 100644 pkg/helpers_test.go diff --git a/cmd/create.go b/cmd/create.go index 13bb0f6d2..093e47e77 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -2,12 +2,12 @@ package cmd import ( "github.com/kubefirst/kubefirst/configs" + "github.com/kubefirst/kubefirst/internal/domain" "github.com/kubefirst/kubefirst/internal/handlers" "github.com/kubefirst/kubefirst/internal/services" "github.com/kubefirst/kubefirst/pkg" "github.com/segmentio/analytics-go" "log" - "net/http" "time" "github.com/kubefirst/kubefirst/internal/gitlab" @@ -49,9 +49,6 @@ cluster provisioning process spinning up the services, and validates the livenes hostedZoneName := viper.GetString("aws.hostedzonename") - // instantiate http client with default values - httpClient := http.DefaultClient - // Instantiates a SegmentIO client to send messages to the segment API. segmentIOClient := analytics.New(pkg.SegmentIOWriteKey) @@ -64,12 +61,20 @@ cluster provisioning process spinning up the services, and validates the livenes } }(segmentIOClient) + telemetryDomain, err := domain.NewTelemetry( + pkg.MetricMgmtClusterInstallStarted, + hostedZoneName, + configs.K1Version, + ) + if err != nil { + log.Println(err) + } telemetryService := services.NewSegmentIoService(segmentIOClient) - telemetryHandler := handlers.NewTelemetry(httpClient, telemetryService) + telemetryHandler := handlers.NewTelemetryHandler(telemetryService) // todo: confirm K1version works for release go-releaser if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(pkg.MetricMgmtClusterInstallStarted, hostedZoneName, configs.K1Version) + err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { log.Println(err) } @@ -106,7 +111,7 @@ cluster provisioning process spinning up the services, and validates the livenes break } } - + informUser("Removing self-signed Argo certificate", globalFlags.SilentMode) clientset, err := k8s.GetClientSet(globalFlags.DryRun) if err != nil { @@ -144,7 +149,7 @@ cluster provisioning process spinning up the services, and validates the livenes // todo: confirm K1version works for release go-releaser if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(pkg.MetricMgmtClusterInstallCompleted, hostedZoneName, configs.K1Version) + err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { log.Println(err) } diff --git a/cmd/init.go b/cmd/init.go index 2f897acb5..c4e9ebe07 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -3,8 +3,8 @@ package cmd import ( "errors" "fmt" + "github.com/kubefirst/kubefirst/internal/domain" "log" - "net/http" "os" "strings" "time" @@ -87,7 +87,7 @@ validated and configured.`, progressPrinter.AddTracker("step-download", pkg.DownloadDependencies, 3) progressPrinter.AddTracker("step-gitops", pkg.CloneAndDetokenizeGitOpsTemplate, 1) progressPrinter.AddTracker("step-ssh", pkg.CreateSSHKey, 1) - progressPrinter.AddTracker("step-telemetry", pkg.SendTelemetry, 1) + progressPrinter.AddTracker("step-telemetryDomain", pkg.SendTelemetry, 1) progressPrinter.SetupProgress(progressPrinter.TotalOfTrackers(), globalFlags.SilentMode) @@ -101,7 +101,6 @@ validated and configured.`, } log.Println("sending init started metric") - httpClient := http.DefaultClient // Instantiates a SegmentIO client to use send messages to the segment API. segmentIOClient := analytics.New(pkg.SegmentIOWriteKey) @@ -115,11 +114,20 @@ validated and configured.`, } }(segmentIOClient) + // validate telemetryDomain data + telemetryDomain, err := domain.NewTelemetry( + pkg.MetricMgmtClusterInstallStarted, + awsFlags.HostedZoneName, + configs.K1Version, + ) + if err != nil { + log.Println(err) + } telemetryService := services.NewSegmentIoService(segmentIOClient) - telemetryHandler := handlers.NewTelemetry(httpClient, telemetryService) + telemetryHandler := handlers.NewTelemetryHandler(telemetryService) // todo: confirm K1version works for release go-releaser if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(pkg.MetricInitStarted, awsFlags.HostedZoneName, configs.K1Version) + err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { log.Println(err) } @@ -211,7 +219,7 @@ validated and configured.`, // todo: confirm K1version works for release go-releaser if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(pkg.MetricInitCompleted, awsFlags.HostedZoneName, configs.K1Version) + err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { log.Println(err) } @@ -220,7 +228,7 @@ validated and configured.`, viper.WriteConfig() //! tracker 8 - progressPrinter.IncrementTracker("step-telemetry", 1) + progressPrinter.IncrementTracker("step-telemetryDomain", 1) time.Sleep(time.Millisecond * 100) informUser("init is done!\n", globalFlags.SilentMode) diff --git a/internal/domain/telemetry.go b/internal/domain/telemetry.go new file mode 100644 index 000000000..c5848d361 --- /dev/null +++ b/internal/domain/telemetry.go @@ -0,0 +1,37 @@ +package domain + +import ( + "errors" + "github.com/kubefirst/kubefirst/pkg" +) + +// Telemetry data that will be consumed by handlers and services +type Telemetry struct { + MetricName string + Domain string + CLIVersion string +} + +// NewTelemetry is the Telemetry domain. When instantiating new Telemetries, we're able to validate domain specific +// values. In this way, domain, handlers and services can work in isolation, and Domain host business logic. +func NewTelemetry(metricName string, domain string, CLIVersion string) (*Telemetry, error) { + + if len(metricName) == 0 { + return nil, errors.New("unable to create metric, missing metric name") + } + + domain, err := pkg.RemoveSubDomain(domain) + if err != nil { + return nil, err + } + + if err = pkg.IsValidURL(domain); err != nil { + return nil, err + } + + return &Telemetry{ + MetricName: metricName, + Domain: domain, + CLIVersion: CLIVersion, + }, nil +} diff --git a/internal/domain/telemetry_test.go b/internal/domain/telemetry_test.go new file mode 100644 index 000000000..09764baaa --- /dev/null +++ b/internal/domain/telemetry_test.go @@ -0,0 +1,76 @@ +package domain + +import ( + "reflect" + "testing" +) + +func TestNewTelemetry(t *testing.T) { + + validTelemetry := Telemetry{MetricName: "test metric", Domain: "example.com", CLIVersion: "0.0.0"} + + type args struct { + metricName string + domain string + CLIVersion string + } + tests := []struct { + name string + args args + want *Telemetry + wantErr bool + }{ + { + name: "valid domain", + args: args{ + metricName: "test metric", + domain: "example.com", + CLIVersion: "0.0.0", + }, + want: &validTelemetry, + wantErr: false, + }, + { + name: "invalid domain", + args: args{ + metricName: "test metric", + domain: "example-com", + CLIVersion: "0.0.0", + }, + want: nil, + wantErr: true, + }, + { + name: "empty domain", + args: args{ + metricName: "test metric", + domain: "", + CLIVersion: "0.0.0", + }, + want: nil, + wantErr: true, + }, + { + name: "missing telemetry name", + args: args{ + metricName: "", + domain: "example.com", + CLIVersion: "0.0.0", + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewTelemetry(tt.args.metricName, tt.args.domain, tt.args.CLIVersion) + if (err != nil) != tt.wantErr { + t.Errorf("NewTelemetry() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("NewTelemetry() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/handlers/telemetry.go b/internal/handlers/telemetry.go index 7a5f10418..d4fff34bd 100644 --- a/internal/handlers/telemetry.go +++ b/internal/handlers/telemetry.go @@ -1,33 +1,30 @@ package handlers import ( - "errors" + "github.com/kubefirst/kubefirst/internal/domain" "github.com/kubefirst/kubefirst/internal/services" - "github.com/kubefirst/kubefirst/pkg" ) // TelemetryHandler hosts handler requirements. type TelemetryHandler struct { - httpClient pkg.HTTPDoer - service services.SegmentIoService + service services.SegmentIoService } -// NewTelemetry instantiate a new Telemetry struct. -func NewTelemetry(httpClient pkg.HTTPDoer, service services.SegmentIoService) TelemetryHandler { +// NewTelemetryHandler instantiate a new Telemetry handler. +func NewTelemetryHandler(service services.SegmentIoService) TelemetryHandler { return TelemetryHandler{ - httpClient: httpClient, - service: service, + service: service, } } // SendCountMetric validate and handles the metric request to the metric service. -func (handler TelemetryHandler) SendCountMetric(metricName string, domain string, cliVersion string) error { +func (handler TelemetryHandler) SendCountMetric(telemetry *domain.Telemetry) error { - if len(metricName) == 0 { - return errors.New("unable to send metric, missing metric name") - } - - err := handler.service.EnqueueCountMetric(metricName, domain, cliVersion) + err := handler.service.EnqueueCountMetric( + telemetry.MetricName, + telemetry.Domain, + telemetry.CLIVersion, + ) if err != nil { return err } diff --git a/internal/handlers/telemetry_test.go b/internal/handlers/telemetry_test.go index 34c6de152..004e0f18d 100644 --- a/internal/handlers/telemetry_test.go +++ b/internal/handlers/telemetry_test.go @@ -1,6 +1,7 @@ package handlers import ( + "github.com/kubefirst/kubefirst/internal/domain" "github.com/kubefirst/kubefirst/internal/services" "github.com/kubefirst/kubefirst/pkg" "reflect" @@ -10,7 +11,6 @@ import ( func TestNewTelemetry(t *testing.T) { // mocks - httpMock := pkg.HTTPMock{} segmentIOMock := pkg.SegmentIOMock{} mockedService := services.SegmentIoService{ @@ -25,19 +25,17 @@ func TestNewTelemetry(t *testing.T) { { name: "newTelemetry", handler: TelemetryHandler{ - httpClient: httpMock, - service: mockedService, + service: mockedService, }, want: TelemetryHandler{ - httpClient: httpMock, - service: mockedService, + service: mockedService, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := NewTelemetry(tt.handler.httpClient, tt.handler.service); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewTelemetry() = %v, want %v", got, tt.want) + if got := NewTelemetryHandler(tt.handler.service); !reflect.DeepEqual(got, tt.want) { + t.Errorf("NewTelemetryHandler() = %v, want %v", got, tt.want) } }) } @@ -45,58 +43,41 @@ func TestNewTelemetry(t *testing.T) { func TestTelemetryHandler_SendCountMetric(t *testing.T) { + validTelemetry := domain.Telemetry{MetricName: "test metric", Domain: "example.com", CLIVersion: "0.0.0"} + // mocks - httpMock := pkg.HTTPMock{} segmentIOMock := pkg.SegmentIOMock{} mockedService := services.SegmentIoService{ SegmentIOClient: segmentIOMock, } + type fields struct { + service services.SegmentIoService + } type args struct { - metricName string - domain string - cliVersion string + telemetry *domain.Telemetry } tests := []struct { name string - handler TelemetryHandler + fields fields args args wantErr bool - }{{ - name: "should pass, its all correct", - handler: TelemetryHandler{ - httpClient: httpMock, - service: mockedService, - }, - args: args{ - metricName: "test-metric-name", - domain: "example.com", - cliVersion: "0.0.1-test", - }, - wantErr: false, - }, + }{ { - name: "should fail when metric name is empty", - handler: TelemetryHandler{ - httpClient: httpMock, - service: mockedService, - }, - args: args{ - metricName: "", - domain: "example.com", - cliVersion: "0.0.1-test", - }, - wantErr: true, - }} + name: "valid telemetry", + fields: fields{service: mockedService}, + args: args{telemetry: &validTelemetry}, + wantErr: false, + }, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { handler := TelemetryHandler{ - httpClient: tt.handler.httpClient, - service: tt.handler.service, + service: tt.fields.service, } - if err := handler.SendCountMetric(tt.args.metricName, tt.args.domain, tt.args.cliVersion); (err != nil) != tt.wantErr { - t.Errorf("EnqueueCountMetric() error = %v, wantErr %v", err, tt.wantErr) + if err := handler.SendCountMetric(tt.args.telemetry); (err != nil) != tt.wantErr { + t.Errorf("SendCountMetric() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/pkg/helpers.go b/pkg/helpers.go index 2e30c055f..a6af3bfe1 100644 --- a/pkg/helpers.go +++ b/pkg/helpers.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "log" "math/rand" + "net/url" "os" "path/filepath" "strings" @@ -307,3 +308,39 @@ func Random(seq int) string { rand.Seed(time.Now().UnixNano()) return randSeq(seq) } + +func RemoveSubDomain(domain string) (string, error) { + + var result string + + splitDomain := strings.Split(domain, ".") + + if len(splitDomain) < 2 { + return "", fmt.Errorf("the domain (%s) is invalid", domain) + } + + if len(splitDomain) == 2 { + return domain, nil + } + + lastURLPart := splitDomain[len(splitDomain)-2:] + domainWithSpace := strings.Join(lastURLPart, " ") + result = strings.ReplaceAll(domainWithSpace, " ", ".") + + err := IsValidURL("https://" + result) + if err != nil { + return "", err + } + + return domain, nil + +} + +func IsValidURL(rawURL string) error { + + parsedURL, err := url.Parse(rawURL) + if err != nil || len(parsedURL.Path) == 0 { + return fmt.Errorf("the URL (%s) is invalid", rawURL) + } + return nil +} diff --git a/pkg/helpers_test.go b/pkg/helpers_test.go new file mode 100644 index 000000000..ae43d5f60 --- /dev/null +++ b/pkg/helpers_test.go @@ -0,0 +1,74 @@ +package pkg + +import ( + "fmt" + "log" + "reflect" + "testing" +) + +func TestRemoveSubDomain(t *testing.T) { + + type args struct { + domain string + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "single domain", + args: args{"example.com"}, + want: "example.com", + wantErr: false, + }, + { + name: "subdomain.domain", + args: args{"hub.example.com"}, + want: "example.com", + wantErr: false, + }, + { + name: "sub.subdomain.domain", + args: args{"hub.hub.example.com"}, + want: "example.com", + wantErr: false, + }, + { + name: "another domain extension", + args: args{"x.xyz"}, + want: "x.xyz", + wantErr: false, + }, + { + name: "invalid domain", + args: args{"xyz"}, + want: "xyz", + wantErr: false, + }, + { + name: "invalid domain", + args: args{"invalid-examplecom"}, + want: "example.com", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := RemoveSubDomain(tt.args.domain) + if (err != nil) != tt.wantErr { + t.Errorf("RemoveSubDomain() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("RemoveSubDomain() got = %v, want %v", got, tt.want) + } + log.Println("---debug---") + fmt.Println(got) + log.Println("---debug---") + + }) + } +} From 378bd47068933772edfb8cfcb5ee543e39cea0ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vanzuita?= Date: Thu, 13 Oct 2022 17:00:41 -0300 Subject: [PATCH 3/5] chore: check http domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Vanzuita --- pkg/helpers.go | 56 +++++++++++++++++++++++---------- pkg/helpers_test.go | 77 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 100 insertions(+), 33 deletions(-) diff --git a/pkg/helpers.go b/pkg/helpers.go index a6af3bfe1..c66b99c49 100644 --- a/pkg/helpers.go +++ b/pkg/helpers.go @@ -309,38 +309,62 @@ func Random(seq int) string { return randSeq(seq) } -func RemoveSubDomain(domain string) (string, error) { +// RemoveSubDomain receives a host and remove its subdomain, if exists. +func RemoveSubDomain(host string) (string, error) { - var result string + // check if received host is valid before parsing it + err := IsValidURL(host) + if err != nil { + return "", err + } - splitDomain := strings.Split(domain, ".") + if !strings.HasPrefix(host, "http") { + return "", errors.New("error...") + } - if len(splitDomain) < 2 { - return "", fmt.Errorf("the domain (%s) is invalid", domain) + // build URL + fullPathURL, err := url.ParseRequestURI(host) + if err != nil { + return "", err } - if len(splitDomain) == 2 { - return domain, nil + splitHost := strings.Split(fullPathURL.Host, ".") + + if len(splitHost) < 2 { + return "", fmt.Errorf("the host (%s) is invalid", host) } - lastURLPart := splitDomain[len(splitDomain)-2:] - domainWithSpace := strings.Join(lastURLPart, " ") - result = strings.ReplaceAll(domainWithSpace, " ", ".") + if len(splitHost) == 2 { + return host, nil + } + + lastURLPart := splitHost[len(splitHost)-2:] + hostWithSpace := strings.Join(lastURLPart, " ") + // set host only without subdomain + fullPathURL.Host = strings.ReplaceAll(hostWithSpace, " ", ".") + + // build URL without subdomain + result := fullPathURL.Scheme + "://" + fullPathURL.Host - err := IsValidURL("https://" + result) + // check if new URL is still valid + err = IsValidURL(result) if err != nil { return "", err } - return domain, nil - + return result, nil } +// IsValidURL checks if a URL is valid func IsValidURL(rawURL string) error { - parsedURL, err := url.Parse(rawURL) - if err != nil || len(parsedURL.Path) == 0 { - return fmt.Errorf("the URL (%s) is invalid", rawURL) + if len(rawURL) == 0 { + return errors.New("rawURL cannot be empty string") + } + + parsedURL, err := url.ParseRequestURI(rawURL) + if err != nil || parsedURL == nil { + return fmt.Errorf("the URL (%s) is invalid, error = %v", rawURL, err) } return nil } diff --git a/pkg/helpers_test.go b/pkg/helpers_test.go index ae43d5f60..71404aa4e 100644 --- a/pkg/helpers_test.go +++ b/pkg/helpers_test.go @@ -1,8 +1,6 @@ package pkg import ( - "fmt" - "log" "reflect" "testing" ) @@ -20,38 +18,38 @@ func TestRemoveSubDomain(t *testing.T) { }{ { name: "single domain", - args: args{"example.com"}, - want: "example.com", + args: args{"https://example.com"}, + want: "https://example.com", wantErr: false, }, { name: "subdomain.domain", - args: args{"hub.example.com"}, - want: "example.com", + args: args{"https://hub.example.com"}, + want: "https://example.com", wantErr: false, }, { name: "sub.subdomain.domain", - args: args{"hub.hub.example.com"}, - want: "example.com", + args: args{"https://hub.hub.example.com"}, + want: "https://example.com", wantErr: false, }, { name: "another domain extension", - args: args{"x.xyz"}, - want: "x.xyz", + args: args{"https://x.xyz"}, + want: "https://x.xyz", wantErr: false, }, { name: "invalid domain", - args: args{"xyz"}, - want: "xyz", - wantErr: false, + args: args{"https://xyz"}, + want: "", + wantErr: true, }, { name: "invalid domain", args: args{"invalid-examplecom"}, - want: "example.com", + want: "", wantErr: true, }, } @@ -65,10 +63,55 @@ func TestRemoveSubDomain(t *testing.T) { if !reflect.DeepEqual(got, tt.want) { t.Errorf("RemoveSubDomain() got = %v, want %v", got, tt.want) } - log.Println("---debug---") - fmt.Println(got) - log.Println("---debug---") + }) + } +} +func Test_isValidURL(t *testing.T) { + type args struct { + rawURL string + } + tests := []struct { + name string + args args + wantErr bool + }{ + { + name: "valid url sample 1", + args: args{rawURL: "https://example.com"}, + wantErr: false, + }, + { + name: "valid url sample 2", + args: args{rawURL: "https://hub.example.com"}, + wantErr: false, + }, + { + name: "empty string", + args: args{rawURL: ""}, + wantErr: true, + }, + { + name: "invalid url sample 1", + args: args{rawURL: "http//example.com"}, + wantErr: true, + }, + { + name: "invalid url sample 2", + args: args{rawURL: "example.com"}, + wantErr: true, + }, + { + name: "invalid url sample 3", + args: args{rawURL: "examplecom"}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := IsValidURL(tt.args.rawURL); (err != nil) != tt.wantErr { + t.Errorf("IsValidURL() error = %v, wantErr %v", err, tt.wantErr) + } }) } } From 8bd3bb96d0cb3408f05672ca2c355b6e19f43620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vanzuita?= Date: Thu, 13 Oct 2022 18:23:55 -0300 Subject: [PATCH 4/5] chore: clean up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Vanzuita --- cmd/create.go | 9 ++++++--- cmd/init.go | 12 ++++++++---- internal/domain/telemetry.go | 4 ---- internal/domain/telemetry_test.go | 4 ++-- pkg/helpers.go | 27 ++++++++++++--------------- pkg/helpers_test.go | 8 ++++---- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/cmd/create.go b/cmd/create.go index 093e47e77..2fb2919ad 100644 --- a/cmd/create.go +++ b/cmd/create.go @@ -72,7 +72,6 @@ cluster provisioning process spinning up the services, and validates the livenes telemetryService := services.NewSegmentIoService(segmentIOClient) telemetryHandler := handlers.NewTelemetryHandler(telemetryService) - // todo: confirm K1version works for release go-releaser if globalFlags.UseTelemetry { err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { @@ -146,10 +145,14 @@ cluster provisioning process spinning up the services, and validates the livenes } log.Println("sending mgmt cluster install completed metric") - // todo: confirm K1version works for release go-releaser + installCompletedTelemetry, err := domain.NewTelemetry( + pkg.MetricMgmtClusterInstallCompleted, + hostedZoneName, + configs.K1Version, + ) if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(telemetryDomain) + err = telemetryHandler.SendCountMetric(installCompletedTelemetry) if err != nil { log.Println(err) } diff --git a/cmd/init.go b/cmd/init.go index c4e9ebe07..c0632834e 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -116,7 +116,7 @@ validated and configured.`, // validate telemetryDomain data telemetryDomain, err := domain.NewTelemetry( - pkg.MetricMgmtClusterInstallStarted, + pkg.MetricInitStarted, awsFlags.HostedZoneName, configs.K1Version, ) @@ -125,7 +125,7 @@ validated and configured.`, } telemetryService := services.NewSegmentIoService(segmentIOClient) telemetryHandler := handlers.NewTelemetryHandler(telemetryService) - // todo: confirm K1version works for release go-releaser + if globalFlags.UseTelemetry { err = telemetryHandler.SendCountMetric(telemetryDomain) if err != nil { @@ -216,10 +216,14 @@ validated and configured.`, progressPrinter.IncrementTracker("step-gitops", 1) log.Println("sending init completed metric") - // todo: confirm K1version works for release go-releaser + telemetryInitCompleted, err := domain.NewTelemetry( + pkg.MetricInitCompleted, + awsFlags.HostedZoneName, + configs.K1Version, + ) if globalFlags.UseTelemetry { - err = telemetryHandler.SendCountMetric(telemetryDomain) + err = telemetryHandler.SendCountMetric(telemetryInitCompleted) if err != nil { log.Println(err) } diff --git a/internal/domain/telemetry.go b/internal/domain/telemetry.go index c5848d361..082aa8f29 100644 --- a/internal/domain/telemetry.go +++ b/internal/domain/telemetry.go @@ -25,10 +25,6 @@ func NewTelemetry(metricName string, domain string, CLIVersion string) (*Telemet return nil, err } - if err = pkg.IsValidURL(domain); err != nil { - return nil, err - } - return &Telemetry{ MetricName: metricName, Domain: domain, diff --git a/internal/domain/telemetry_test.go b/internal/domain/telemetry_test.go index 09764baaa..5a12873b2 100644 --- a/internal/domain/telemetry_test.go +++ b/internal/domain/telemetry_test.go @@ -24,7 +24,7 @@ func TestNewTelemetry(t *testing.T) { name: "valid domain", args: args{ metricName: "test metric", - domain: "example.com", + domain: "https://example.com", CLIVersion: "0.0.0", }, want: &validTelemetry, @@ -34,7 +34,7 @@ func TestNewTelemetry(t *testing.T) { name: "invalid domain", args: args{ metricName: "test metric", - domain: "example-com", + domain: "https://example-com", CLIVersion: "0.0.0", }, want: nil, diff --git a/pkg/helpers.go b/pkg/helpers.go index c66b99c49..0b4a2561f 100644 --- a/pkg/helpers.go +++ b/pkg/helpers.go @@ -310,20 +310,21 @@ func Random(seq int) string { } // RemoveSubDomain receives a host and remove its subdomain, if exists. -func RemoveSubDomain(host string) (string, error) { +func RemoveSubDomain(fullURL string) (string, error) { - // check if received host is valid before parsing it - err := IsValidURL(host) - if err != nil { - return "", err + // add http if fullURL doesn't have it, this is for validation only, won't be used on http requests + if !strings.HasPrefix(fullURL, "http") { + fullURL = "https://" + fullURL } - if !strings.HasPrefix(host, "http") { - return "", errors.New("error...") + // check if received fullURL is valid before parsing it + err := IsValidURL(fullURL) + if err != nil { + return "", err } // build URL - fullPathURL, err := url.ParseRequestURI(host) + fullPathURL, err := url.ParseRequestURI(fullURL) if err != nil { return "", err } @@ -331,16 +332,12 @@ func RemoveSubDomain(host string) (string, error) { splitHost := strings.Split(fullPathURL.Host, ".") if len(splitHost) < 2 { - return "", fmt.Errorf("the host (%s) is invalid", host) - } - - if len(splitHost) == 2 { - return host, nil + return "", fmt.Errorf("the fullURL (%s) is invalid", fullURL) } lastURLPart := splitHost[len(splitHost)-2:] hostWithSpace := strings.Join(lastURLPart, " ") - // set host only without subdomain + // set fullURL only without subdomain fullPathURL.Host = strings.ReplaceAll(hostWithSpace, " ", ".") // build URL without subdomain @@ -352,7 +349,7 @@ func RemoveSubDomain(host string) (string, error) { return "", err } - return result, nil + return fullPathURL.Host, nil } // IsValidURL checks if a URL is valid diff --git a/pkg/helpers_test.go b/pkg/helpers_test.go index 71404aa4e..c9572f1a4 100644 --- a/pkg/helpers_test.go +++ b/pkg/helpers_test.go @@ -19,25 +19,25 @@ func TestRemoveSubDomain(t *testing.T) { { name: "single domain", args: args{"https://example.com"}, - want: "https://example.com", + want: "example.com", wantErr: false, }, { name: "subdomain.domain", args: args{"https://hub.example.com"}, - want: "https://example.com", + want: "example.com", wantErr: false, }, { name: "sub.subdomain.domain", args: args{"https://hub.hub.example.com"}, - want: "https://example.com", + want: "example.com", wantErr: false, }, { name: "another domain extension", args: args{"https://x.xyz"}, - want: "https://x.xyz", + want: "x.xyz", wantErr: false, }, { From 5557b5010123867a9da30cb76dca1bf892b37d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vanzuita?= Date: Thu, 13 Oct 2022 18:32:08 -0300 Subject: [PATCH 5/5] chore: clean up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Vanzuita --- cmd/init.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/init.go b/cmd/init.go index c0632834e..5e60bd3c0 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -87,7 +87,7 @@ validated and configured.`, progressPrinter.AddTracker("step-download", pkg.DownloadDependencies, 3) progressPrinter.AddTracker("step-gitops", pkg.CloneAndDetokenizeGitOpsTemplate, 1) progressPrinter.AddTracker("step-ssh", pkg.CreateSSHKey, 1) - progressPrinter.AddTracker("step-telemetryDomain", pkg.SendTelemetry, 1) + progressPrinter.AddTracker("step-telemetry", pkg.SendTelemetry, 1) progressPrinter.SetupProgress(progressPrinter.TotalOfTrackers(), globalFlags.SilentMode) @@ -232,7 +232,7 @@ validated and configured.`, viper.WriteConfig() //! tracker 8 - progressPrinter.IncrementTracker("step-telemetryDomain", 1) + progressPrinter.IncrementTracker("step-telemetry", 1) time.Sleep(time.Millisecond * 100) informUser("init is done!\n", globalFlags.SilentMode)