Skip to content

Commit

Permalink
OCM-7110 | fix: GitHub IDP validation
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
mnecas committed Apr 2, 2024
1 parent c9f520d commit 89c1c88
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
9 changes: 4 additions & 5 deletions cmd/create/idp/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions pkg/interactive/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 13 additions & 0 deletions pkg/interactive/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})

})

0 comments on commit 89c1c88

Please sign in to comment.