From 89c1c88b4525c7fc7875f48ba7febccd9f568316 Mon Sep 17 00:00:00 2001 From: Martin Necas Date: Tue, 2 Apr 2024 15:20:27 +0200 Subject: [PATCH] OCM-7110 | fix: GitHub IDP validation Switch from URI validation to the DNS1123 validation. The openshift does not accept the URI format of domains. So when user specifies some address with prefix https:// it will fail. Signed-off-by: Martin Necas --- cmd/create/idp/github.go | 9 ++++----- pkg/interactive/validation.go | 18 ++++++++++++++++++ pkg/interactive/validation_test.go | 13 +++++++++++++ 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cmd/create/idp/github.go b/cmd/create/idp/github.go index 06198129a4..7b2fe31a24 100644 --- a/cmd/create/idp/github.go +++ b/cmd/create/idp/github.go @@ -24,10 +24,9 @@ import ( "strings" cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1" - "github.com/spf13/cobra" - "github.com/openshift/rosa/pkg/interactive" "github.com/openshift/rosa/pkg/ocm" + "github.com/spf13/cobra" ) func buildGithubIdp(cmd *cobra.Command, @@ -190,7 +189,7 @@ func buildGithubIdp(cmd *cobra.Command, Help: cmd.Flags().Lookup("hostname").Usage, Default: githubHostname, Validators: []interactive.Validator{ - interactive.IsURL, + interactive.IsValidHostname, }, }) if err != nil { @@ -201,9 +200,9 @@ func buildGithubIdp(cmd *cobra.Command, return idpBuilder, fmt.Errorf("CA is not expected when not using a hosted instance of Github Enterprise") } if githubHostname != "" { - _, err = url.ParseRequestURI(githubHostname) + err := interactive.IsValidHostname(githubHostname) if err != nil { - return idpBuilder, fmt.Errorf("Expected a valid Hostname: %s", err) + return idpBuilder, err } // Set the hostname, if any githubIDP = githubIDP.Hostname(githubHostname) diff --git a/pkg/interactive/validation.go b/pkg/interactive/validation.go index 712c3d6b1c..42e9b18ff9 100644 --- a/pkg/interactive/validation.go +++ b/pkg/interactive/validation.go @@ -26,6 +26,9 @@ import ( "regexp" "strconv" + "k8s.io/apimachinery/pkg/util/validation" + netutils "k8s.io/utils/net" + "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/core" clustervalidations "github.com/openshift-online/ocm-common/pkg/cluster/validations" @@ -59,6 +62,21 @@ func IsURL(val interface{}) error { return err } +func IsValidHostname(val interface{}) error { + if !_isValidHostname(val.(string)) { + return fmt.Errorf(fmt.Sprintf("'%s' hostname must be a valid DNS subdomain or IP address", val.(string))) + } + return nil +} + +// _isValidHostname is same validation as in the Open Shift GitHub IDP CRD +// https://github.com/openshift/kubernetes/blob/91607f5d750ba4002f87d34a12ae1cfd45b45b81/openshift-kube-apiserver/admission/customresourcevalidation/oauth/helpers.go#L13 +// +//nolint:lll +func _isValidHostname(hostname string) bool { + return len(validation.IsDNS1123Subdomain(hostname)) == 0 || netutils.ParseIPSloppy(hostname) != nil +} + func IsURLHttps(val interface{}) error { parsedUri, err := _isUrl(val) if err != nil { diff --git a/pkg/interactive/validation_test.go b/pkg/interactive/validation_test.go index 73750aed30..11bd9834c7 100644 --- a/pkg/interactive/validation_test.go +++ b/pkg/interactive/validation_test.go @@ -77,5 +77,18 @@ var _ = Describe("Validation", func() { Expect(err).NotTo(HaveOccurred()) }) }) + Context("GitHub Hostname", func() { + It("Fails validation if hostname is 'https://github.com'", func() { + err := IsValidHostname("https://github.com") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring( + "'https://github.com' hostname must be a valid DNS subdomain or IP address"), + ) + }) + It("Passes validation if hostname is 'domain.customer.com'", func() { + err := IsValidHostname("domain.customer.com") + Expect(err).NotTo(HaveOccurred()) + }) + }) })