From 7b2e292e9bf9d357e540a079555a08b516be04be Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 3 Nov 2021 17:19:26 +0000 Subject: [PATCH 1/6] fix: use remote spec for terms and conditions --- Makefile | 5 ++-- internal/build/build.go | 7 ++--- pkg/ams/TermsAndConditionsSpec.go | 12 +++++++++ pkg/ams/ams.go | 7 +++-- pkg/ams/terms.go | 43 +++++++++++++++++++++++++++++++ pkg/cmd/kafka/create/create.go | 3 ++- pkg/cmd/registry/create/create.go | 5 ++-- 7 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 pkg/ams/TermsAndConditionsSpec.go create mode 100644 pkg/ams/terms.go diff --git a/Makefile b/Makefile index 8c13521f0..6413619db 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ RHOAS_VERSION ?= "dev" REPOSITORY_OWNER ?= "redhat-developer" REPOSITORY_NAME ?= "app-services-cli" TERMS_REVIEW_EVENT_CODE ?= "register" -TERMS_REVIEW_SITE_CODE ?= "ocm" +TERMS_SPEC_URL ?= "https://raw.githubusercontent.com/redhat-developer/app-services-ui/6699c93c9d5f123a2c898d1a1d72a9c290a20820/static/configs/terms-conditions-spec.json" SSO_REDIRECT_PATH ?= "sso-redhat-callback" MAS_SSO_REDIRECT_PATH ?= "mas-sso-callback" @@ -17,8 +17,7 @@ DEFAULT_PAGE_SIZE ?= "10" GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.Version=$(RHOAS_VERSION) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryOwner=$(REPOSITORY_OWNER) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.RepositoryName=$(REPOSITORY_NAME) $(GO_LDFLAGS) -GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewEventCode=$(TERMS_REVIEW_EVENT_CODE) $(GO_LDFLAGS) -GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewSiteCode=$(TERMS_REVIEW_SITE_CODE) $(GO_LDFLAGS) +GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.TermsReviewSpecURL=$(TERMS_SPEC_URL) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.DefaultPageSize=$(DEFAULT_PAGE_SIZE) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.DefaultPageNumber=$(DEFAULT_PAGE_NUMBER) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/redhat-developer/app-services-cli/internal/build.SSORedirectPath=$(SSO_REDIRECT_PATH) $(GO_LDFLAGS) diff --git a/internal/build/build.go b/internal/build/build.go index 094ff180f..f669d3720 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -25,11 +25,8 @@ var ( // RepositoryName is the remote GitHub repository for the releases RepositoryName = "app-services-cli" - // TermsReviewEventCode is the event code used when checking the terms review - TermsReviewEventCode = "register" - - // TermsReviewSiteCode is the site code used when checking the terms review - TermsReviewSiteCode = "ocm" + // TermsReviewSpecURL Url used to download terms and conditions specification + TermsReviewSpecURL = "https://raw.githubusercontent.com/redhat-developer/app-services-ui/6699c93c9d5f123a2c898d1a1d72a9c290a20820/static/configs/terms-conditions-spec.json" // DefaultPageSize is the default number of items per page when using list commands DefaultPageSize = "10" diff --git a/pkg/ams/TermsAndConditionsSpec.go b/pkg/ams/TermsAndConditionsSpec.go new file mode 100644 index 000000000..cec7451b9 --- /dev/null +++ b/pkg/ams/TermsAndConditionsSpec.go @@ -0,0 +1,12 @@ +package ams + +type TermsAndConditionsSpec struct { + Kafka ServiceTermsSpec `json:"kafka"` + ServiceRegistry ServiceTermsSpec `json:"service-registry"` +} + +type ServiceTermsSpec struct { + EventCode string `json:"EventCode"` + SiteCode string `json:"SiteCode"` + StopOnTermsChange bool `json:"StopOnTermsChange"` +} diff --git a/pkg/ams/ams.go b/pkg/ams/ams.go index 59cf3f850..b293e829c 100644 --- a/pkg/ams/ams.go +++ b/pkg/ams/ams.go @@ -4,17 +4,16 @@ import ( "context" "errors" - "github.com/redhat-developer/app-services-cli/internal/build" "github.com/redhat-developer/app-services-cli/pkg/api/ams/amsclient" "github.com/redhat-developer/app-services-cli/pkg/connection" ) -func CheckTermsAccepted(ctx context.Context, conn connection.Connection) (accepted bool, redirectURI string, err error) { +func CheckTermsAccepted(ctx context.Context, spec ServiceTermsSpec, conn connection.Connection) (accepted bool, redirectURI string, err error) { termsReview, _, err := conn.API().AccountMgmt(). ApiAuthorizationsV1SelfTermsReviewPost(ctx). SelfTermsReview(amsclient.SelfTermsReview{ - EventCode: &build.TermsReviewEventCode, - SiteCode: &build.TermsReviewSiteCode, + EventCode: &spec.EventCode, + SiteCode: &spec.SiteCode, }). Execute() if err != nil { diff --git a/pkg/ams/terms.go b/pkg/ams/terms.go new file mode 100644 index 000000000..3d71058fa --- /dev/null +++ b/pkg/ams/terms.go @@ -0,0 +1,43 @@ +package ams + +import ( + "encoding/json" + "io/ioutil" + "net/http" + + "github.com/redhat-developer/app-services-cli/internal/build" +) + +var fallbackTocSpec = []byte(`{ + "kafka":{ + "EventCode":"register", + "SiteCode":"ocm", + "StopOnTermsChange": true + }, + "service-registry":{ + "EventCode":"onlineService", + "SiteCode":"ocm", + "StopOnTermsChange": true + } + } +`) + +func GetRemoteTermsSpec() TermsAndConditionsSpec { + response, err := http.Get(build.TermsReviewSpecURL) + + var specJson []byte + if err != nil { + // TODO log error? + specJson = fallbackTocSpec + } else { + specJson, err = ioutil.ReadAll(response.Body) + if err != nil { + // TODO log error? + specJson = fallbackTocSpec + } + } + + var termsAndConditionsSpec TermsAndConditionsSpec + json.Unmarshal([]byte(specJson), &termsAndConditionsSpec) + return termsAndConditionsSpec +} diff --git a/pkg/cmd/kafka/create/create.go b/pkg/cmd/kafka/create/create.go index 398bafcfa..63e6297a3 100644 --- a/pkg/cmd/kafka/create/create.go +++ b/pkg/cmd/kafka/create/create.go @@ -145,7 +145,8 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a kafka instance - termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, conn) + termsSpec := ams.GetRemoteTermsSpec() + termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.Kafka, conn) if err != nil { return err } diff --git a/pkg/cmd/registry/create/create.go b/pkg/cmd/registry/create/create.go index 97856b8d7..4c86778fb 100644 --- a/pkg/cmd/registry/create/create.go +++ b/pkg/cmd/registry/create/create.go @@ -4,12 +4,12 @@ import ( "context" "fmt" + "github.com/redhat-developer/app-services-cli/pkg/ams" "github.com/redhat-developer/app-services-cli/pkg/icon" "github.com/redhat-developer/app-services-cli/pkg/localize" "github.com/redhat-developer/app-services-cli/pkg/serviceregistry" - "github.com/redhat-developer/app-services-cli/pkg/ams" "github.com/redhat-developer/app-services-cli/pkg/cmd/flag" flagutil "github.com/redhat-developer/app-services-cli/pkg/cmdutil/flagutil" "github.com/redhat-developer/app-services-cli/pkg/connection" @@ -119,7 +119,8 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a registry instance - termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, conn) + termsSpec := ams.GetRemoteTermsSpec() + termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.ServiceRegistry, conn) if err != nil { return err } From fb59e87a08936a5fa02fd9980bbd1260bf893157 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Wed, 3 Nov 2021 17:26:35 +0000 Subject: [PATCH 2/6] fix: swap to main branch --- Makefile | 2 +- internal/build/build.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6413619db..5d8b5a252 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ RHOAS_VERSION ?= "dev" REPOSITORY_OWNER ?= "redhat-developer" REPOSITORY_NAME ?= "app-services-cli" TERMS_REVIEW_EVENT_CODE ?= "register" -TERMS_SPEC_URL ?= "https://raw.githubusercontent.com/redhat-developer/app-services-ui/6699c93c9d5f123a2c898d1a1d72a9c290a20820/static/configs/terms-conditions-spec.json" +TERMS_SPEC_URL ?= "https://raw.githubusercontent.com/redhat-developer/app-services-ui/main/static/configs/terms-conditions-spec.json" SSO_REDIRECT_PATH ?= "sso-redhat-callback" MAS_SSO_REDIRECT_PATH ?= "mas-sso-callback" diff --git a/internal/build/build.go b/internal/build/build.go index f669d3720..b87a93312 100644 --- a/internal/build/build.go +++ b/internal/build/build.go @@ -26,7 +26,7 @@ var ( RepositoryName = "app-services-cli" // TermsReviewSpecURL Url used to download terms and conditions specification - TermsReviewSpecURL = "https://raw.githubusercontent.com/redhat-developer/app-services-ui/6699c93c9d5f123a2c898d1a1d72a9c290a20820/static/configs/terms-conditions-spec.json" + TermsReviewSpecURL = "https://raw.githubusercontent.com/redhat-developer/app-services-ui/main/static/configs/terms-conditions-spec.json" // DefaultPageSize is the default number of items per page when using list commands DefaultPageSize = "10" From 956e0bde7e2c3d097bc6b81dedb1b8c13e61a1d2 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 4 Nov 2021 10:00:33 +0000 Subject: [PATCH 3/6] fix: refactorings of the flow --- Makefile | 1 - pkg/ams/terms.go | 58 ++++++++++++++++++------------- pkg/cmd/kafka/create/create.go | 2 +- pkg/cmd/registry/create/create.go | 2 +- 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 5d8b5a252..70c11350b 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,6 @@ SHELL = bash RHOAS_VERSION ?= "dev" REPOSITORY_OWNER ?= "redhat-developer" REPOSITORY_NAME ?= "app-services-cli" -TERMS_REVIEW_EVENT_CODE ?= "register" TERMS_SPEC_URL ?= "https://raw.githubusercontent.com/redhat-developer/app-services-ui/main/static/configs/terms-conditions-spec.json" SSO_REDIRECT_PATH ?= "sso-redhat-callback" MAS_SSO_REDIRECT_PATH ?= "mas-sso-callback" diff --git a/pkg/ams/terms.go b/pkg/ams/terms.go index 3d71058fa..34300d166 100644 --- a/pkg/ams/terms.go +++ b/pkg/ams/terms.go @@ -6,38 +6,48 @@ import ( "net/http" "github.com/redhat-developer/app-services-cli/internal/build" + "github.com/redhat-developer/app-services-cli/pkg/logging" ) -var fallbackTocSpec = []byte(`{ - "kafka":{ - "EventCode":"register", - "SiteCode":"ocm", - "StopOnTermsChange": true - }, - "service-registry":{ - "EventCode":"onlineService", - "SiteCode":"ocm", - "StopOnTermsChange": true - } - } -`) - -func GetRemoteTermsSpec() TermsAndConditionsSpec { +// Contains specification for terms and condition parameters +// NOTE: Before updating this fallback file +// Please update source at https://github.com/redhat-developer/app-services-ui/blob/main/static/configs/terms-conditions-spec.json +var fallbackTocSpec = TermsAndConditionsSpec{ + Kafka: ServiceTermsSpec{ + EventCode: "register", + SiteCode: "ocm", + }, + ServiceRegistry: ServiceTermsSpec{ + EventCode: "onlineService", + SiteCode: "ocm", + }, +} + +// GetRemoteTermsSpec fetch event and site code information associated with the services +// Function is used to dynamically download new terms and conditions specifications +// without forcing end users to update their CLI. +func GetRemoteTermsSpec(logger logging.Logger) TermsAndConditionsSpec { + response, err := http.Get(build.TermsReviewSpecURL) + if err != nil || response.Body == nil { + logger.Debug("Fetching remote terms failed with error ", err) + return fallbackTocSpec + } + defer response.Body.Close() + var specJson []byte + specJson, err = ioutil.ReadAll(response.Body) if err != nil { - // TODO log error? - specJson = fallbackTocSpec - } else { - specJson, err = ioutil.ReadAll(response.Body) - if err != nil { - // TODO log error? - specJson = fallbackTocSpec - } + logger.Debug("Reading remote terms failed with error ", err) + return fallbackTocSpec } var termsAndConditionsSpec TermsAndConditionsSpec - json.Unmarshal([]byte(specJson), &termsAndConditionsSpec) + err = json.Unmarshal([]byte(specJson), &termsAndConditionsSpec) + if err != nil { + logger.Debug("Parsing remote terms failed with error ", err) + return fallbackTocSpec + } return termsAndConditionsSpec } diff --git a/pkg/cmd/kafka/create/create.go b/pkg/cmd/kafka/create/create.go index 63e6297a3..4b68f805e 100644 --- a/pkg/cmd/kafka/create/create.go +++ b/pkg/cmd/kafka/create/create.go @@ -145,7 +145,7 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a kafka instance - termsSpec := ams.GetRemoteTermsSpec() + termsSpec := ams.GetRemoteTermsSpec(opts.Logger) termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.Kafka, conn) if err != nil { return err diff --git a/pkg/cmd/registry/create/create.go b/pkg/cmd/registry/create/create.go index 4c86778fb..9134f56b5 100644 --- a/pkg/cmd/registry/create/create.go +++ b/pkg/cmd/registry/create/create.go @@ -119,7 +119,7 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a registry instance - termsSpec := ams.GetRemoteTermsSpec() + termsSpec := ams.GetRemoteTermsSpec(opts.Logger) termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.ServiceRegistry, conn) if err != nil { return err From 73a46043786f4cc4f11d15f37d1e4e331bc0d2c6 Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 4 Nov 2021 10:14:00 +0000 Subject: [PATCH 4/6] fix: add support for timetouts --- pkg/ams/terms.go | 16 ++++++++++------ pkg/cmd/kafka/create/create.go | 2 +- pkg/cmd/registry/create/create.go | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/ams/terms.go b/pkg/ams/terms.go index 34300d166..1fad28290 100644 --- a/pkg/ams/terms.go +++ b/pkg/ams/terms.go @@ -1,6 +1,7 @@ package ams import ( + "context" "encoding/json" "io/ioutil" "net/http" @@ -26,18 +27,21 @@ var fallbackTocSpec = TermsAndConditionsSpec{ // GetRemoteTermsSpec fetch event and site code information associated with the services // Function is used to dynamically download new terms and conditions specifications // without forcing end users to update their CLI. -func GetRemoteTermsSpec(logger logging.Logger) TermsAndConditionsSpec { - - response, err := http.Get(build.TermsReviewSpecURL) - +func GetRemoteTermsSpec(context *context.Context, logger logging.Logger) TermsAndConditionsSpec { + client := &http.Client{} + req, err := http.NewRequestWithContext(*context, http.MethodGet, build.TermsReviewSpecURL, nil) + if err != nil { + logger.Debug("Fetching remote terms failed with error ", err) + return fallbackTocSpec + } + response, err := client.Do(req) if err != nil || response.Body == nil { logger.Debug("Fetching remote terms failed with error ", err) return fallbackTocSpec } defer response.Body.Close() - var specJson []byte - specJson, err = ioutil.ReadAll(response.Body) + specJson, err := ioutil.ReadAll(response.Body) if err != nil { logger.Debug("Reading remote terms failed with error ", err) return fallbackTocSpec diff --git a/pkg/cmd/kafka/create/create.go b/pkg/cmd/kafka/create/create.go index 4b68f805e..babd686c6 100644 --- a/pkg/cmd/kafka/create/create.go +++ b/pkg/cmd/kafka/create/create.go @@ -145,7 +145,7 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a kafka instance - termsSpec := ams.GetRemoteTermsSpec(opts.Logger) + termsSpec := ams.GetRemoteTermsSpec(&opts.Context, opts.Logger) termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.Kafka, conn) if err != nil { return err diff --git a/pkg/cmd/registry/create/create.go b/pkg/cmd/registry/create/create.go index 9134f56b5..5197c2320 100644 --- a/pkg/cmd/registry/create/create.go +++ b/pkg/cmd/registry/create/create.go @@ -119,7 +119,7 @@ func runCreate(opts *options) error { // the user must have accepted the terms and conditions from the provider // before they can create a registry instance - termsSpec := ams.GetRemoteTermsSpec(opts.Logger) + termsSpec := ams.GetRemoteTermsSpec(&opts.Context, opts.Logger) termsAccepted, termsURL, err := ams.CheckTermsAccepted(opts.Context, termsSpec.ServiceRegistry, conn) if err != nil { return err From abda51d12f44f40bc4b3165f94bd5df19a115d2e Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 4 Nov 2021 10:47:54 +0000 Subject: [PATCH 5/6] fix: add verification steps --- .cases/termsandconditions.sh | 20 ++++++++++++++++++++ pkg/ams/TermsAndConditionsSpec.go | 5 ++--- pkg/ams/terms.go | 2 ++ 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 .cases/termsandconditions.sh diff --git a/.cases/termsandconditions.sh b/.cases/termsandconditions.sh new file mode 100644 index 000000000..7f6fddbbe --- /dev/null +++ b/.cases/termsandconditions.sh @@ -0,0 +1,20 @@ +#!/bin/bash + +## Terms and Conditions acceptance + +## Requirements +## Create new account without any terms and conditions accepted. + +## Framework + +alias rhoas=$(go env GOPATH)/bin/rhoas + +## Cases + +rhoas service-registry create --name=test -v +## In order to be able to create a new instance, you must first review and accept the terms and conditions: +## https://www.redhat.com/wapps/tnc/ackrequired?site=ocm&event=register + +rhoas kafka create --name=test --provider=aws --region=eu-east1 -v +## In order to be able to create a new instance, you must first review and accept the terms and conditions: +## https://www.redhat.com/wapps/tnc/ackrequired?site=ocm&event=onlineService \ No newline at end of file diff --git a/pkg/ams/TermsAndConditionsSpec.go b/pkg/ams/TermsAndConditionsSpec.go index cec7451b9..02408c92d 100644 --- a/pkg/ams/TermsAndConditionsSpec.go +++ b/pkg/ams/TermsAndConditionsSpec.go @@ -6,7 +6,6 @@ type TermsAndConditionsSpec struct { } type ServiceTermsSpec struct { - EventCode string `json:"EventCode"` - SiteCode string `json:"SiteCode"` - StopOnTermsChange bool `json:"StopOnTermsChange"` + EventCode string `json:"EventCode"` + SiteCode string `json:"SiteCode"` } diff --git a/pkg/ams/terms.go b/pkg/ams/terms.go index 1fad28290..634de7100 100644 --- a/pkg/ams/terms.go +++ b/pkg/ams/terms.go @@ -47,6 +47,8 @@ func GetRemoteTermsSpec(context *context.Context, logger logging.Logger) TermsAn return fallbackTocSpec } + logger.Debug("Terms spec: ", specJson) + var termsAndConditionsSpec TermsAndConditionsSpec err = json.Unmarshal([]byte(specJson), &termsAndConditionsSpec) if err != nil { From 26d3a959db0f40d222573c92e2b86d24e5165d3d Mon Sep 17 00:00:00 2001 From: Wojciech Trocki Date: Thu, 4 Nov 2021 13:14:42 +0000 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Enda --- pkg/ams/terms.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ams/terms.go b/pkg/ams/terms.go index 634de7100..fd51f1a7b 100644 --- a/pkg/ams/terms.go +++ b/pkg/ams/terms.go @@ -31,11 +31,11 @@ func GetRemoteTermsSpec(context *context.Context, logger logging.Logger) TermsAn client := &http.Client{} req, err := http.NewRequestWithContext(*context, http.MethodGet, build.TermsReviewSpecURL, nil) if err != nil { - logger.Debug("Fetching remote terms failed with error ", err) + logger.Debug("Fetching remote terms failed with error", err) return fallbackTocSpec } response, err := client.Do(req) - if err != nil || response.Body == nil { + if err != nil || response == nil { logger.Debug("Fetching remote terms failed with error ", err) return fallbackTocSpec }