Skip to content

Commit

Permalink
Revert "fix(kafka create): reduce number of calls made to AMS API (#1596
Browse files Browse the repository at this point in the history
)" (#1616)

This reverts commit 5346fc1.
  • Loading branch information
wtrocki authored Jun 28, 2022
1 parent 5346fc1 commit 955dd6f
Show file tree
Hide file tree
Showing 11 changed files with 218 additions and 160 deletions.
1 change: 0 additions & 1 deletion docs/commands/rhoas_kafka_create.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ require (
github.com/redhat-developer/app-services-sdk-go/accountmgmt v0.2.0
github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.7.0
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1
github.com/redhat-developer/app-services-sdk-go/registrymgmt v0.6.1
github.com/redhat-developer/service-binding-operator v0.9.0
Expand Down
5 changes: 2 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,8 @@ github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.7.0 h1:GcbNg/Ad
github.com/redhat-developer/app-services-sdk-go/connectormgmt v0.7.0/go.mod h1:0WB4LlMmesjBlGKvnMXQ7twPxeSr27f5a+w4QnMoSdQ=
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0 h1:ExEHQaihnPNxN2nKXB0q5nrmSv4p8b3Idzt7TChxv+Q=
github.com/redhat-developer/app-services-sdk-go/kafkainstance v0.6.0/go.mod h1:hMpejngP3BFnifCDH1gKRG9cU9Q4lr0WiQaW7A1LYo4=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1 h1:Gcyn2kLlslsVT6T8qoiCJpJFPrnD2i2KIFeKQJrXkTY=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.1/go.mod h1:RoPo3tyHjv8apStFNVjChwWYdlWhg6hMzi1IrH3yQX8=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0 h1:63UhOYB8TozKdnkkws2pXc0D1lEB+K3qX63/OxkjDas=
github.com/redhat-developer/app-services-sdk-go/kafkamgmt v0.12.0/go.mod h1:m+m7d6xkC9WbSxemslyhjv0jVhquWLysRfdh+RQ5hH0=
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1 h1:xRq5XJzRDs/Z7e/9SDt6zbNRIyesC4LTqN9ajHKwjHo=
github.com/redhat-developer/app-services-sdk-go/registryinstance v0.3.1/go.mod h1:Z/gr/snlpsqYg4vftmcx97vCR3qMQJhALGelDHx4pMA=
github.com/redhat-developer/app-services-sdk-go/registrymgmt v0.6.1 h1:3sUmQ3nAawsYWg7ZCO2Q8HF2J7MW6YA38h/YFL3ao6o=
Expand Down Expand Up @@ -933,7 +933,6 @@ golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f/go.mod h1:KelEdhl1UZF7XfJ
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220524215830-622c5d57e401/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb/go.mod h1:jaDAt6Dkxork7LmZnYtzbRWj0W47D86a3TGe0YHBvmE=
golang.org/x/oauth2 v0.0.0-20220622183110-fd043fe589d2 h1:+jnHzr9VPj32ykQVai5DNahi9+NSp7yYuCsl5eAQtL0=
golang.org/x/oauth2 v0.0.0-20220622183110-fd043fe589d2/go.mod h1:jaDAt6Dkxork7LmZnYtzbRWj0W47D86a3TGe0YHBvmE=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down
28 changes: 5 additions & 23 deletions pkg/cmd/kafka/create/api_validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package create
import (
"strings"

"github.com/redhat-developer/app-services-cli/pkg/core/cmdutil/flagutil"
"github.com/redhat-developer/app-services-cli/pkg/core/localize"
"github.com/redhat-developer/app-services-cli/pkg/shared/accountmgmtutil"
"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
Expand All @@ -14,18 +13,17 @@ import (
)

type ValidatorInput struct {
provider string
region string
size string
provider string
region string
size string

userAMSInstanceType *accountmgmtutil.QuotaSpec

f *factory.Factory
constants *remote.DynamicServiceConstants
conn connection.Connection
}

var validBillingModels []string = []string{accountmgmtutil.QuotaMarketplaceType, accountmgmtutil.QuotaStandardType}

func (input *ValidatorInput) ValidateProviderAndRegion() error {
f := input.f
f.Logger.Debug("Validating provider and region")
Expand Down Expand Up @@ -110,7 +108,7 @@ func (input *ValidatorInput) ValidateSize() error {
return nil
}

sizes, err := FetchValidKafkaSizesLabels(input.f, input.provider, input.region, input.userAMSInstanceType)
sizes, err := FetchValidKafkaSizesLabels(input.f, input.provider, input.region, *input.userAMSInstanceType)
if err != nil {
return err
}
Expand All @@ -121,19 +119,3 @@ func (input *ValidatorInput) ValidateSize() error {

return nil
}

// ValidateBillingModel validates if user provided a correct billing model
func ValidateBillingModel(billingModel string) error {

if billingModel == "" {
return nil
}

isValid := flagutil.IsValidInput(billingModel, validBillingModels...)

if isValid {
return nil
}

return flagutil.InvalidValueError("billing-model", billingModel, validBillingModels...)
}
48 changes: 48 additions & 0 deletions pkg/cmd/kafka/create/completions.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package create

import (
"github.com/redhat-developer/app-services-cli/pkg/shared/accountmgmtutil"
"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
"github.com/redhat-developer/app-services-cli/pkg/shared/factory"
"github.com/redhat-developer/app-services-cli/pkg/shared/remote"
"github.com/spf13/cobra"
)

Expand All @@ -23,3 +26,48 @@ func GetCloudProviderRegionCompletionValues(f *factory.Factory, providerID strin

return validRegions, cobra.ShellCompDirectiveNoSpace
}

// GetKafkaSizeCompletionValues returns a list of valid kafka sizes for the specified region and ams instance types
func GetKafkaSizeCompletionValues(f *factory.Factory, providerID string, regionId string) (validRegions []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

// We need both values to provide a valid list of sizes
if providerID == "" || regionId == "" {
return nil, directive
}

err, constants := remote.GetRemoteServiceConstants(f.Context, f.Logger)
if err != nil {
return nil, directive
}

conn, err := f.Connection(connection.DefaultConfigSkipMasAuth)
if err != nil {
return nil, directive
}

userInstanceType, _ := accountmgmtutil.GetUserSupportedInstanceType(f.Context, &constants.Kafka.Ams, conn)

// Not including quota in this request as it takes very long time to list quota for all regions in suggestion mode
validRegions, _ = FetchValidKafkaSizesLabels(f, providerID, regionId, *userInstanceType)

return validRegions, cobra.ShellCompDirectiveNoSpace
}

// GetMarketplaceAcctIdCompletionValues returns a list of valid marketplace account IDs for the organization
func GetMarketplaceAcctIdCompletionValues(f *factory.Factory) (validMarketplaceAcctIDs []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

validMarketplaceAcctIDs, _ = accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, "")

return validMarketplaceAcctIDs, directive
}

// GetMarketplaceCompletionValues returns a list of valid marketplaces for the organization
func GetMarketplaceCompletionValues(f *factory.Factory) (validMarketplaces []string, directive cobra.ShellCompDirective) {
directive = cobra.ShellCompDirectiveNoSpace

validMarketplaces, _ = accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)

return validMarketplaces, directive
}
95 changes: 56 additions & 39 deletions pkg/cmd/kafka/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ type options struct {

marketplaceAcctId string
marketplace string
billingModel string

outputFormat string
autoUse bool
Expand Down Expand Up @@ -94,7 +93,7 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
}
}

if opts.bypassChecks && (opts.marketplace != "" || opts.marketplaceAcctId != "" || opts.billingModel != "") {
if opts.bypassChecks && (opts.marketplace != "" || opts.marketplaceAcctId != "") {
return f.Localizer.MustLocalizeError("kafka.create.error.bypassChecks.marketplace")
}

Expand All @@ -112,6 +111,26 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
return flagutil.InvalidValueError("output", opts.outputFormat, validOutputFormats...)
}

if !opts.bypassChecks {
validMarketplaces, err := accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)
if err != nil {
return err
}

if opts.marketplace != "" && !flagutil.IsValidInput(opts.marketplace, validMarketplaces...) {
return flagutil.InvalidValueError(FlagMarketPlace, opts.marketplace, validMarketplaces...)
}

validMarketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, opts.marketplace)
if err != nil {
return err
}

if opts.marketplaceAcctId != "" && !flagutil.IsValidInput(opts.marketplaceAcctId, validMarketplaceAcctIDs...) {
return flagutil.InvalidValueError(FlagMarketPlaceAcctID, opts.marketplaceAcctId, validMarketplaceAcctIDs...)
}
}

return runCreate(opts)
},
}
Expand All @@ -128,7 +147,6 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
flags.BoolVar(&opts.autoUse, "use", true, f.Localizer.MustLocalize("kafka.create.flag.autoUse.description"))
flags.BoolVarP(&opts.wait, "wait", "w", false, f.Localizer.MustLocalize("kafka.create.flag.wait.description"))
flags.BoolVarP(&opts.dryRun, "dry-run", "", false, f.Localizer.MustLocalize("kafka.create.flag.dryrun.description"))
flags.StringVar(&opts.billingModel, "billing-model", "", f.Localizer.MustLocalize("kafka.create.flag.billingModel.description"))
flags.AddBypassTermsCheck(&opts.bypassChecks)

_ = cmd.RegisterFlagCompletionFunc(FlagProvider, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
Expand All @@ -139,6 +157,18 @@ func NewCreateCommand(f *factory.Factory) *cobra.Command {
return GetCloudProviderRegionCompletionValues(f, opts.provider)
})

_ = cmd.RegisterFlagCompletionFunc(FlagSize, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetKafkaSizeCompletionValues(f, opts.provider, opts.region)
})

_ = cmd.RegisterFlagCompletionFunc(FlagMarketPlaceAcctID, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetMarketplaceAcctIdCompletionValues(f)
})

_ = cmd.RegisterFlagCompletionFunc(FlagMarketPlace, func(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {
return GetMarketplaceCompletionValues(f)
})

return cmd
}

Expand Down Expand Up @@ -182,16 +212,10 @@ func runCreate(opts *options) error {
return nil
}

err = ValidateBillingModel(opts.billingModel)
if err != nil {
return err
userInstanceType, err = accountmgmtutil.GetUserSupportedInstanceType(f.Context, &constants.Kafka.Ams, conn)
if err != nil || userInstanceType == nil {
return f.Localizer.MustLocalizeError("kafka.create.error.userInstanceType.notFound")
}

userInstanceType, err = accountmgmtutil.FetchQuotaCost(f, opts.billingModel, opts.marketplaceAcctId, opts.marketplace, &constants.Kafka.Ams)
if err != nil {
return err
}

}

var payload *kafkamgmtclient.KafkaRequestPayload
Expand All @@ -201,7 +225,7 @@ func runCreate(opts *options) error {
return f.Localizer.MustLocalizeError("kafka.create.error.noInteractiveMode")
}

payload, err = promptKafkaPayload(opts, userInstanceType)
payload, err = promptKafkaPayload(opts, *userInstanceType)
if err != nil {
return err
}
Expand All @@ -225,8 +249,6 @@ func runCreate(opts *options) error {
payload.Marketplace.Set(&opts.marketplace)
payload.BillingCloudAccountId = kafkamgmtclient.NullableString{}
payload.BillingCloudAccountId.Set(&opts.marketplaceAcctId)
payload.BillingModel = kafkamgmtclient.NullableString{}
payload.BillingModel.Set(&opts.billingModel)
}

if !opts.bypassChecks {
Expand All @@ -249,7 +271,7 @@ func runCreate(opts *options) error {
return err1
}
if opts.size != "" {
sizes, err1 := FetchValidKafkaSizes(opts.f, opts.provider, opts.region, userInstanceType)
sizes, err1 := FetchValidKafkaSizes(opts.f, opts.provider, opts.region, *userInstanceType)
if err1 != nil {
return err1
}
Expand Down Expand Up @@ -378,7 +400,7 @@ type promptAnswers struct {

// Show a prompt to allow the user to interactively insert the data for their Kafka
// nolint:funlen
func promptKafkaPayload(opts *options, userQuotaType *accountmgmtutil.QuotaSpec) (*kafkamgmtclient.KafkaRequestPayload, error) {
func promptKafkaPayload(opts *options, userQuotaType accountmgmtutil.QuotaSpec) (*kafkamgmtclient.KafkaRequestPayload, error) {
f := opts.f

accountIDNullable := kafkamgmtclient.NullableString{}
Expand Down Expand Up @@ -416,7 +438,7 @@ func promptKafkaPayload(opts *options, userQuotaType *accountmgmtutil.QuotaSpec)
return nil, err
}

regionIDs, err := GetEnabledCloudRegionIDs(opts.f, answers.CloudProvider, userQuotaType)
regionIDs, err := GetEnabledCloudRegionIDs(opts.f, answers.CloudProvider, &userQuotaType)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -452,34 +474,29 @@ func promptKafkaPayload(opts *options, userQuotaType *accountmgmtutil.QuotaSpec)
}
}

if !opts.bypassChecks && opts.billingModel == accountmgmtutil.QuotaMarketplaceType {
marketplaces, err := accountmgmtutil.GetValidMarketplaces(f.Context, f.Connection)
if err != nil {
return nil, err
}

marketplaces, err := accountmgmtutil.GetValidMarketplaces(userQuotaType)
if err != nil {
if !opts.bypassChecks && len(marketplaces) > 0 {
if err = promptMarketplaceSelect(f.Localizer, marketplaces, answers); err != nil {
return nil, err
}

if len(marketplaces) > 0 {

if err = promptMarketplaceSelect(f.Localizer, marketplaces, answers); err != nil {
return nil, err
}
marketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(f.Context, f.Connection, answers.Marketplace)
if err != nil {
return nil, err
}

marketplaceAcctIDs, err := accountmgmtutil.GetValidMarketplaceAcctIDs(userQuotaType, answers.Marketplace)
if err != nil {
if len(marketplaceAcctIDs) > 0 {
if err = promptMarketplaceAcctIDSelect(f.Localizer, marketplaceAcctIDs, answers); err != nil {
return nil, err
}

if len(marketplaceAcctIDs) > 0 {
if err = promptMarketplaceAcctIDSelect(f.Localizer, marketplaceAcctIDs, answers); err != nil {
return nil, err
}
}

accountIDNullable.Set(&answers.MarketplaceAcctID)
marketplaceProviderNullable.Set(&answers.Marketplace)

}

accountIDNullable.Set(&answers.MarketplaceAcctID)
marketplaceProviderNullable.Set(&answers.Marketplace)
}

payload := &kafkamgmtclient.KafkaRequestPayload{
Expand All @@ -490,7 +507,7 @@ func promptKafkaPayload(opts *options, userQuotaType *accountmgmtutil.QuotaSpec)
Marketplace: marketplaceProviderNullable,
}
printSizeWarningIfNeeded(opts.f, answers.Size, sizes)
payload.SetPlan(mapAmsTypeToBackendType(userQuotaType) + "." + answers.Size)
payload.SetPlan(mapAmsTypeToBackendType(&userQuotaType) + "." + answers.Size)

return payload, nil
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cmd/kafka/create/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func GetValidKafkaSizesLabels(sizes []kafkamgmtclient.SupportedKafkaSize) []stri
}

func FetchValidKafkaSizesLabels(f *factory.Factory,
providerID string, regionId string, amsType *accountmgmtutil.QuotaSpec) ([]string, error) {
providerID string, regionId string, amsType accountmgmtutil.QuotaSpec) ([]string, error) {
sizes, err := FetchValidKafkaSizes(f, providerID, regionId, amsType)
if err != nil {
return nil, err
Expand All @@ -56,7 +56,7 @@ func FetchValidKafkaSizesLabels(f *factory.Factory,

// return list of the valid instance sizes for the specified region and ams instance types
func FetchValidKafkaSizes(f *factory.Factory,
providerID string, regionId string, amsType *accountmgmtutil.QuotaSpec) ([]kafkamgmtclient.SupportedKafkaSize, error) {
providerID string, regionId string, amsType accountmgmtutil.QuotaSpec) ([]kafkamgmtclient.SupportedKafkaSize, error) {

conn, err := f.Connection(connection.DefaultConfigSkipMasAuth)
if err != nil {
Expand All @@ -73,7 +73,7 @@ func FetchValidKafkaSizes(f *factory.Factory,
return nil, err
}

desiredInstanceType := mapAmsTypeToBackendType(amsType)
desiredInstanceType := mapAmsTypeToBackendType(&amsType)

// Temporary workaround to be removed
if desiredInstanceType == DeveloperType {
Expand Down
7 changes: 0 additions & 7 deletions pkg/cmd/kafka/kafkacmdutil/kafka_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@ import (
kafkamgmtclient "github.com/redhat-developer/app-services-sdk-go/kafkamgmt/apiv1/client"
)

var (
billingModelMarketplace = "marketplace"
billingModelStandard = "standard"
)

var ValidBillingModels = []string{billingModelMarketplace, billingModelStandard}

var (
validNameRegexp = regexp.MustCompile(`^[a-z]([-a-z0-9]*[a-z0-9])?$`)
validSearchRegexp = regexp.MustCompile(`^([a-zA-Z0-9-_%]*[a-zA-Z0-9-_%])?$`)
Expand Down
Loading

0 comments on commit 955dd6f

Please sign in to comment.