review improvements

cli/cmd/get.go

@@ -354,10 +354,10 @@ func apiSplitterTable(apiSplitter *schema.APISplitter, env cliconfig.Environment
 return out, nil
 }
 
-func trafficSplitTable(trafficSplitter schema.APISplitter, env cliconfig.Environment) (table.Table, error) {
-rows := make([][]interface{}, 0, len(trafficSplitter.Spec.APIs))
+func trafficSplitTable(apiSplitter schema.APISplitter, env cliconfig.Environment) (table.Table, error) {
+rows := make([][]interface{}, 0, len(apiSplitter.Spec.APIs))
 
-for _, api := range trafficSplitter.Spec.APIs {
+for _, api := range apiSplitter.Spec.APIs {
 apiRes, err := cluster.GetAPI(MustGetOperatorConfig(env.Name), api.Name)
 if err != nil {
 return table.Table{}, err
@@ -392,10 +392,10 @@ func trafficSplitTable(trafficSplitter schema.APISplitter, env cliconfig.Environ
 }, nil
 }
 
-func apiSplitterListTable(trafficSplitter []schema.APISplitter, envNames []string) table.Table {
-rows := make([][]interface{}, 0, len(trafficSplitter))
+func apiSplitterListTable(apiSplitter []schema.APISplitter, envNames []string) table.Table {
+rows := make([][]interface{}, 0, len(apiSplitter))
 
-for i, splitAPI := range trafficSplitter {
+for i, splitAPI := range apiSplitter {
 lastUpdated := time.Unix(splitAPI.Spec.LastUpdated, 0)
 var apis []string
 for _, api := range splitAPI.Spec.APIs {

pkg/operator/resources/apisplitter/api.go

@@ -54,7 +54,7 @@ func UpdateAPI(apiConfig *userconfig.API, projectID string, force bool) (*spec.A
 return api, fmt.Sprintf("created %s", api.Name), nil
 }
 
-if !areVirtualServiceEqual(prevVirtualService, virtualServiceSpec(api, getTrafficSplitterDestinations(api))) {
+if !areVirtualServiceEqual(prevVirtualService, virtualServiceSpec(api)) {
 if err := config.AWS.UploadMsgpackToS3(api, config.Cluster.Bucket, api.Key); err != nil {
 return nil, "", errors.Wrap(err, "upload api spec")
 }
@@ -119,8 +119,8 @@ func applyK8sResources(api *spec.API, prevVirtualService *istioclientnetworking.
 return applyK8sVirtualService(api, prevVirtualService)
 }
 
-func applyK8sVirtualService(trafficsplitter *spec.API, prevVirtualService *istioclientnetworking.VirtualService) error {
-newVirtualService := virtualServiceSpec(trafficsplitter, getTrafficSplitterDestinations(trafficsplitter))
+func applyK8sVirtualService(apiSplitter *spec.API, prevVirtualService *istioclientnetworking.VirtualService) error {
+newVirtualService := virtualServiceSpec(apiSplitter)
 
 if prevVirtualService == nil {
 _, err := config.K8s.CreateVirtualService(newVirtualService)
@@ -131,9 +131,9 @@ func applyK8sVirtualService(trafficsplitter *spec.API, prevVirtualService *istio
 return err
 }
 
-func getTrafficSplitterDestinations(trafficsplitter *spec.API) []k8s.Destination {
-destinations := make([]k8s.Destination, len(trafficsplitter.APIs))
-for i, api := range trafficsplitter.APIs {
+func getAPISplitterDestinations(apiSplitter *spec.API) []k8s.Destination {
+destinations := make([]k8s.Destination, len(apiSplitter.APIs))
+for i, api := range apiSplitter.APIs {
 destinations[i] = k8s.Destination{
 ServiceName: operator.K8sName(api.Name),
 Weight: int32(api.Weight),

pkg/operator/resources/apisplitter/k8s_specs.go

@@ -29,20 +29,20 @@ const (
 _defaultPortInt32, _defaultPortStr = int32(8888), "8888"
 )
 
-func virtualServiceSpec(trafficsplitter *spec.API, destinations []k8s.Destination) *istioclientnetworking.VirtualService {
+func virtualServiceSpec(apiSplitter *spec.API) *istioclientnetworking.VirtualService {
 return k8s.VirtualService(&k8s.VirtualServiceSpec{
-Name: operator.K8sName(trafficsplitter.Name),
+Name: operator.K8sName(apiSplitter.Name),
 Gateways: []string{"apis-gateway"},
-Destinations: destinations,
-Path: *trafficsplitter.Networking.Endpoint,
+Destinations: getAPISplitterDestinations(apiSplitter),
+Path: *apiSplitter.Networking.Endpoint,
 Rewrite: pointer.String("predict"),
 Annotations: map[string]string{
-userconfig.EndpointAnnotationKey: *trafficsplitter.Networking.Endpoint,
-userconfig.APIGatewayAnnotationKey: trafficsplitter.Networking.APIGateway.String()},
+userconfig.EndpointAnnotationKey: *apiSplitter.Networking.Endpoint,
+userconfig.APIGatewayAnnotationKey: apiSplitter.Networking.APIGateway.String()},
 Labels: map[string]string{
-"apiName": trafficsplitter.Name,
-"apiKind": trafficsplitter.Kind.String(),
-"apiID": trafficsplitter.ID,
+"apiName": apiSplitter.Name,
+"apiKind": apiSplitter.Kind.String(),
+"apiID": apiSplitter.ID,
 },
 })
 }

pkg/operator/resources/apisplitter/status.go

@@ -37,7 +37,7 @@ func GetStatus(apiName string) (*status.Status, error) {
 return nil, errors.ErrorUnexpected("unable to find trafficsplitter", apiName)
 }
 
-return trafficSplitterStatus(virtualService)
+return apiSplitterStatus(virtualService)
 }
 
 func GetAllStatuses() ([]status.Status, error) {
@@ -50,7 +50,7 @@ func GetAllStatuses() ([]status.Status, error) {
 
 statuses := make([]status.Status, len(virtualServices))
 for i, virtualService := range virtualServices {
-status, err := trafficSplitterStatus(&virtualService)
+status, err := apiSplitterStatus(&virtualService)
 if err != nil {
 return nil, err
 }
@@ -63,12 +63,12 @@ func GetAllStatuses() ([]status.Status, error) {
 return statuses, nil
 }
 
-func trafficSplitterStatus(virtualService *istioclientnetworking.VirtualService) (*status.Status, error) {
+func apiSplitterStatus(virtualService *istioclientnetworking.VirtualService) (*status.Status, error) {
 
 statusResponse := &status.Status{}
 statusResponse.APIName = virtualService.Labels["apiName"]
 statusResponse.APIID = virtualService.Labels["apiID"]
-// if virtual service deploy the trafficsplitter is actice
+// if virtual service deploy the trafficsplitter is active
 // maybe need to check if backends are active
 statusResponse.Code = status.Live
 

pkg/operator/resources/errors.go

@@ -30,7 +30,7 @@ const (
 ErrCannotChangeTypeOfDeployedAPI = "resources.cannot_change_kind_of_deployed_api"
 ErrNoAvailableNodeComputeLimit = "resources.no_available_node_compute_limit"
 ErrAPIUsedByAPISplitter = "resources.syncapi_used_by_apisplitter"
-ErrNotDeployedAPIsAPISplitter = "resources.trafficsplit_apis_not_deplyoed"
+ErrNotDeployedAPIsAPISplitter = "resources.trafficsplit_apis_not_deployed"
 )
 
 func ErrorOperationNotSupportedForKind(kind userconfig.Kind) error {
@@ -68,7 +68,7 @@ func ErrorNoAvailableNodeComputeLimit(resource string, reqStr string, maxStr str
 func ErrorAPIUsedByAPISplitter(apiSplitters []string) error {
 return errors.WithStack(&errors.Error{
 Kind: ErrAPIUsedByAPISplitter,
-Message: fmt.Sprintf("can not delete api because it is used by APISplitter: %s", strings.StrsSentence(apiSplitters, "")),
+Message: fmt.Sprintf("cannot delete api because it is used by the following %s: %s", strings.PluralS("APISplitter", len(apiSplitters)), strings.StrsSentence(apiSplitters, "")),
 })
 }
 

pkg/operator/resources/resources.go

@@ -74,26 +74,26 @@ func Deploy(projectBytes []byte, configFileName string, configBytes []byte, forc
 ConfigFileName: configFileName,
 }
 
-isProjectUploaded, err := config.AWS.IsS3File(config.Cluster.Bucket, projectKey)
+apiConfigs, err := spec.ExtractAPIConfigs(configBytes, types.AWSProviderType, configFileName)
 if err != nil {
 return nil, err
 }
-if !isProjectUploaded {
-if err = config.AWS.UploadBytesToS3(projectBytes, config.Cluster.Bucket, projectKey); err != nil {
-return nil, err
-}
-}
 
-apiConfigs, err := spec.ExtractAPIConfigs(configBytes, types.AWSProviderType, configFileName)
+err = ValidateClusterAPIs(apiConfigs, projectFiles)
 if err != nil {
+err = errors.Append(err, fmt.Sprintf("\n\napi configuration schema can be found here: https://docs.cortex.dev/v/%s/deployments/api-configuration", consts.CortexVersionMinor))
 return nil, err
 }
 
-err = ValidateClusterAPIs(apiConfigs, projectFiles)
+isProjectUploaded, err := config.AWS.IsS3File(config.Cluster.Bucket, projectKey)
 if err != nil {
-err = errors.Append(err, fmt.Sprintf("\n\napi configuration schema can be found here: https://docs.cortex.dev/v/%s/deployments/api-configuration", consts.CortexVersionMinor))
 return nil, err
 }
+if !isProjectUploaded {
+if err = config.AWS.UploadBytesToS3(projectBytes, config.Cluster.Bucket, projectKey); err != nil {
+return nil, err
+}
+}
 
 // order apiconfigs first syncAPIs then TrafficSplit
 // This is done if user specifies SyncAPIs in same file as APISplitter
@@ -247,17 +247,17 @@ func GetAPIs() (*schema.GetAPIsResponse, error) {
 }
 
 // get all apiSplitters
-apiSplitterstatuses, err := apisplitter.GetAllStatuses()
+apiSplitterStatuses, err := apisplitter.GetAllStatuses()
 if err != nil {
 return nil, err
 }
-if len(apiSplitterstatuses) > 0 {
-apiSplitterapiNames, apiSplitterapiIDs := namesAndIDsFromStatuses(apiSplitterstatuses)
-apiSplitterapis, err := operator.DownloadAPISpecs(apiSplitterapiNames, apiSplitterapiIDs)
+if len(apiSplitterStatuses) > 0 {
+apiSplitterAPINames, apiSplitterAPIIDs := namesAndIDsFromStatuses(apiSplitterStatuses)
+apiSplitterAPIs, err := operator.DownloadAPISpecs(apiSplitterAPINames, apiSplitterAPIIDs)
 if err != nil {
 return nil, err
 }
-for _, api := range apiSplitterapis {
+for _, api := range apiSplitterAPIs {
 apiSplitter = append(apiSplitter, schema.APISplitter{
 Spec: api,
 })
@@ -340,16 +340,22 @@ func namesAndIDsFromStatuses(statuses []status.Status) ([]string, []string) {
 return apiNames, apiIDs
 }
 
+//checkIfUsedByAPISplitter checks if api is used by a deployed APISplitter
 func checkIfUsedByAPISplitter(apiName string) error {
-apiRes, err := GetAPIs()
+virtualServices, err := config.K8s.ListVirtualServicesByLabel("apiKind", userconfig.APISplitterKind.String())
 if err != nil {
 return err
 }
+
 var usedByAPISplitters []string
-for _, apiSplitter := range apiRes.APISplitter {
-for _, api := range apiSplitter.Spec.APIs {
+for _, vs := range virtualServices {
+apiSplitterSpec, err := operator.DownloadAPISpec(vs.Labels["apiName"], vs.Labels["apiID"])
+if err != nil {
+return err
+}
+for _, api := range apiSplitterSpec.APIs {
 if apiName == api.Name {
-usedByAPISplitters = append(usedByAPISplitters, apiSplitter.Spec.Name)
+usedByAPISplitters = append(usedByAPISplitters, apiSplitterSpec.Name)
 }
 }
 }

pkg/operator/resources/validations.go

@@ -228,12 +228,7 @@ func findDuplicateEndpoints(apis []userconfig.API) []userconfig.API {
 endpoints := make(map[string][]userconfig.API)
 
 for _, api := range apis {
-if api.Kind == userconfig.SyncAPIKind {
-endpoints[*api.Networking.Endpoint] = append(endpoints[*api.Networking.Endpoint], api)
-}
-if api.Kind == userconfig.APISplitterKind {
-endpoints[*api.Networking.Endpoint] = append(endpoints[*api.Networking.Endpoint], api)
-}
+endpoints[*api.Networking.Endpoint] = append(endpoints[*api.Networking.Endpoint], api)
 }
 
 for endpoint := range endpoints {

pkg/operator/schema/schema.go

@@ -53,7 +53,7 @@ type DeployResult struct {
 
 type GetAPIsResponse struct {
 SyncAPIs []SyncAPI `json:"sync_apis"`
-APISplitter []APISplitter `json:"api_splitter"`
+APISplitter []APISplitter `json:"api_splitters"`
 }
 
 type SyncAPI struct {

pkg/types/spec/validations.go

@@ -54,7 +54,7 @@ func apiValidation(provider types.ProviderType, resource userconfig.Resource) *c
 if resource.Kind == userconfig.SyncAPIKind {
 structFieldValidations = append(structFieldValidations,
 predictorValidation(),
-networkingValidation(),
+networkingValidation(resource.Kind),
 computeValidation(provider),
 monitoringValidation(),
 autoscalingValidation(provider),
@@ -64,7 +64,7 @@ func apiValidation(provider types.ProviderType, resource userconfig.Resource) *c
 if resource.Kind == userconfig.APISplitterKind {
 structFieldValidations = append(structFieldValidations,
 multiAPIsValidation(),
-networkingValidationNoLocal(),
+networkingValidation(resource.Kind),
 )
 }
 return &cr.StructValidation{
@@ -122,33 +122,6 @@ func multiAPIsValidation() *cr.StructFieldValidation {
 }
 }
 
-func networkingValidationNoLocal() *cr.StructFieldValidation {
-return &cr.StructFieldValidation{
-StructField: "Networking",
-StructValidation: &cr.StructValidation{
-StructFieldValidations: []*cr.StructFieldValidation{
-{
-StructField: "Endpoint",
-StringPtrValidation: &cr.StringPtrValidation{
-Validator: urls.ValidateEndpoint,
-MaxLength: 1000, // no particular reason other than it works
-},
-},
-{
-StructField: "APIGateway",
-StringValidation: &cr.StringValidation{
-AllowedValues: userconfig.APIGatewayTypeStrings(),
-Default: userconfig.PublicAPIGatewayType.String(),
-},
-Parser: func(str string) (interface{}, error) {
-return userconfig.APIGatewayTypeFromString(str), nil
-},
-},
-},
-},
-}
-}
-
 func predictorValidation() *cr.StructFieldValidation {
 return &cr.StructFieldValidation{
 StructField: "Predictor",
@@ -275,36 +248,39 @@ func monitoringValidation() *cr.StructFieldValidation {
 }
 }
 
-func networkingValidation() *cr.StructFieldValidation {
+func networkingValidation(kind userconfig.Kind) *cr.StructFieldValidation {
+structFieldValidation := []*cr.StructFieldValidation{
+{
+StructField: "Endpoint",
+StringPtrValidation: &cr.StringPtrValidation{
+Validator: urls.ValidateEndpoint,
+MaxLength: 1000, // no particular reason other than it works
+},
+},
+{
+StructField: "APIGateway",
+StringValidation: &cr.StringValidation{
+AllowedValues: userconfig.APIGatewayTypeStrings(),
+Default: userconfig.PublicAPIGatewayType.String(),
+},
+Parser: func(str string) (interface{}, error) {
+return userconfig.APIGatewayTypeFromString(str), nil
+},
+},
+}
+if kind == userconfig.SyncAPIKind {
+structFieldValidation = append(structFieldValidation, &cr.StructFieldValidation{
+StructField: "LocalPort",
+IntPtrValidation: &cr.IntPtrValidation{
+GreaterThan: pointer.Int(0),
+LessThanOrEqualTo: pointer.Int(math.MaxUint16),
+},
+})
+}
 return &cr.StructFieldValidation{
 StructField: "Networking",
 StructValidation: &cr.StructValidation{
-StructFieldValidations: []*cr.StructFieldValidation{
-{
-StructField: "Endpoint",
-StringPtrValidation: &cr.StringPtrValidation{
-Validator: urls.ValidateEndpoint,
-MaxLength: 1000, // no particular reason other than it works
-},
-},
-{
-StructField: "LocalPort",
-IntPtrValidation: &cr.IntPtrValidation{
-GreaterThan: pointer.Int(0),
-LessThanOrEqualTo: pointer.Int(math.MaxUint16),
-},
-},
-{
-StructField: "APIGateway",
-StringValidation: &cr.StringValidation{
-AllowedValues: userconfig.APIGatewayTypeStrings(),
-Default: userconfig.PublicAPIGatewayType.String(),
-},
-Parser: func(str string) (interface{}, error) {
-return userconfig.APIGatewayTypeFromString(str), nil
-},
-},
-},
+StructFieldValidations: structFieldValidation,
 },
 }
 }