Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load service model name from generator #216

Merged
merged 4 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions cmd/ack-generate/command/apis.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
ackmetadata "github.com/aws-controllers-k8s/code-generator/pkg/metadata"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
"github.com/aws-controllers-k8s/code-generator/pkg/util"
)

Expand Down Expand Up @@ -97,25 +96,11 @@ func generateAPIs(cmd *cobra.Command, args []string) error {
if err := ensureSDKRepo(ctx, optCacheDir, optRefreshCache); err != nil {
return err
}
sdkHelper := ackmodel.NewSDKHelper(sdkDir)
sdkAPI, err := sdkHelper.API(svcAlias)
if err != nil {
newSvcAlias, err := FallBackFindServiceID(sdkDir, svcAlias)
if err != nil {
return err
}
sdkAPI, err = sdkHelper.API(newSvcAlias) // retry with serviceID
if err != nil {
return fmt.Errorf("service %s not found", svcAlias)
}
}
model, err := ackmodel.New(
sdkAPI, optGenVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
m, err := loadModelWithLatestAPIVersion(svcAlias)
if err != nil {
return err
}
ts, err := ackgenerate.APIs(model, optTemplateDirs)
ts, err := ackgenerate.APIs(m, optTemplateDirs)
if err != nil {
return err
}
Expand Down
70 changes: 70 additions & 0 deletions cmd/ack-generate/command/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ import (
"os"
"os/signal"
"path/filepath"
"sort"
"strings"
"syscall"
"time"

"golang.org/x/mod/modfile"

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
"github.com/aws-controllers-k8s/code-generator/pkg/util"
k8sversion "k8s.io/apimachinery/pkg/version"
)

const (
Expand Down Expand Up @@ -207,3 +212,68 @@ func getSDKVersionFromGoMod(goModPath string) (string, error) {
}
return "", fmt.Errorf("couldn't find %s in the go.mod require block", sdkModule)
}

// loadModelWithLatestAPIVersion finds the AWS SDK for a given service alias and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the code below, considering (in a future PR if you agree with me) perhaps putting this into an importable package under pkg/

// creates a new model with the latest API version.
func loadModelWithLatestAPIVersion(svcAlias string) (*ackmodel.Model, error) {
latestAPIVersion, err := getLatestAPIVersion()
if err != nil {
return nil, err
}
return loadModel(svcAlias, latestAPIVersion)
}

// loadModel finds the AWS SDK for a given service alias and creates a new model
// with the given API version.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if apiVersion is empty string?

func loadModel(svcAlias string, apiVersion string) (*ackmodel.Model, error) {
cfg, err := ackgenconfig.New(optGeneratorConfigPath, ackgenerate.DefaultConfig)
if err != nil {
return nil, err
}

modelName := strings.ToLower(cfg.ModelName)
if modelName == "" {
modelName = svcAlias
}

sdkHelper := ackmodel.NewSDKHelper(sdkDir)
sdkAPI, err := sdkHelper.API(modelName)
if err != nil {
retryModelName, err := FallBackFindServiceID(sdkDir, svcAlias)
if err != nil {
return nil, err
}
// Retry using path found by querying service ID
sdkAPI, err = sdkHelper.API(retryModelName)
if err != nil {
return nil, fmt.Errorf("service %s not found", svcAlias)
}
}
m, err := ackmodel.New(
sdkAPI, svcAlias, apiVersion, cfg,
)
if err != nil {
return nil, err
}
return m, nil
}

// getLatestAPIVersion looks in a target output directory to determine what the
// latest Kubernetes API version for CRDs exposed by the generated service
// controller.
func getLatestAPIVersion() (string, error) {
apisPath := filepath.Join(optOutputPath, "apis")
versions := []string{}
subdirs, err := ioutil.ReadDir(apisPath)
if err != nil {
return "", err
}

for _, subdir := range subdirs {
versions = append(versions, subdir.Name())
}
sort.Slice(versions, func(i, j int) bool {
return k8sversion.CompareKubeAwareVersionStrings(versions[i], versions[j]) < 0
})
return versions[len(versions)-1], nil
}
43 changes: 1 addition & 42 deletions cmd/ack-generate/command/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,11 @@ import (
"os"
"path/filepath"
"regexp"
"sort"
"strings"

"github.com/spf13/cobra"
k8sversion "k8s.io/apimachinery/pkg/version"

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
)

var (
Expand Down Expand Up @@ -62,25 +59,7 @@ func generateController(cmd *cobra.Command, args []string) error {
if err := ensureSDKRepo(ctx, optCacheDir, optRefreshCache); err != nil {
return err
}
sdkHelper := ackmodel.NewSDKHelper(sdkDir)
sdkAPI, err := sdkHelper.API(svcAlias)
if err != nil {
newSvcAlias, err := FallBackFindServiceID(sdkDir, svcAlias)
if err != nil {
return err
}
sdkAPI, err = sdkHelper.API(newSvcAlias) // retry with serviceID
if err != nil {
return fmt.Errorf("service %s not found", svcAlias)
}
}
latestAPIVersion, err = getLatestAPIVersion()
if err != nil {
return err
}
m, err := ackmodel.New(
sdkAPI, latestAPIVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
m, err := loadModelWithLatestAPIVersion(svcAlias)
if err != nil {
return err
}
Expand Down Expand Up @@ -111,26 +90,6 @@ func generateController(cmd *cobra.Command, args []string) error {
return nil
}

// getLatestAPIVersion looks in a target output directory to determine what the
// latest Kubernetes API version for CRDs exposed by the generated service
// controller.
func getLatestAPIVersion() (string, error) {
apisPath := filepath.Join(optOutputPath, "apis")
versions := []string{}
subdirs, err := ioutil.ReadDir(apisPath)
if err != nil {
return "", err
}

for _, subdir := range subdirs {
versions = append(versions, subdir.Name())
}
sort.Slice(versions, func(i, j int) bool {
return k8sversion.CompareKubeAwareVersionStrings(versions[i], versions[j]) < 0
})
return versions[len(versions)-1], nil
}

// FallBackFindServiceID reads through aws-sdk-go/models/apis/*/*/api-2.json
// Returns ServiceID (as newSuppliedAlias) if supplied service Alias matches with serviceID in api-2.json
// If not a match, return the supllied alias.
Expand Down
24 changes: 15 additions & 9 deletions cmd/ack-generate/command/crossplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"github.com/pkg/errors"
"github.com/spf13/cobra"

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
cpgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/crossplane"
"github.com/aws-controllers-k8s/code-generator/pkg/model"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
Expand Down Expand Up @@ -56,6 +58,18 @@ func generateCrossplane(_ *cobra.Command, args []string) error {
return err
}
svcAlias := strings.ToLower(args[0])
cfgPath := filepath.Join(providerDir, "apis", svcAlias, optGenVersion, "generator-config.yaml")
_, err := os.Stat(cfgPath)
if err != nil && !os.IsNotExist(err) {
return err
}
if os.IsNotExist(err) {
cfgPath = ""
}
cfg, err := ackgenconfig.New(cfgPath, ackgenerate.DefaultConfig)
if err != nil {
return err
}
sdkHelper := model.NewSDKHelper(sdkDir)
sdkHelper.APIGroupSuffix = "aws.crossplane.io"
sdkAPI, err := sdkHelper.API(svcAlias)
Expand All @@ -69,16 +83,8 @@ func generateCrossplane(_ *cobra.Command, args []string) error {
return fmt.Errorf("cannot get the API model for service %s", svcAlias)
}
}
cfgPath := filepath.Join(providerDir, "apis", svcAlias, optGenVersion, "generator-config.yaml")
_, err = os.Stat(cfgPath)
if err != nil && !os.IsNotExist(err) {
return err
}
if os.IsNotExist(err) {
cfgPath = ""
}
m, err := ackmodel.New(
sdkAPI, optGenVersion, cfgPath, cpgenerate.DefaultConfig,
sdkAPI, svcAlias, optGenVersion, cfg,
)
if err != nil {
return err
Expand Down
23 changes: 1 addition & 22 deletions cmd/ack-generate/command/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ import (
"github.com/ghodss/yaml"
"github.com/spf13/cobra"

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
olmgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/olm"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
)

const (
Expand Down Expand Up @@ -86,26 +84,7 @@ func generateOLMAssets(cmd *cobra.Command, args []string) error {
if err := ensureSDKRepo(ctx, optCacheDir, optRefreshCache); err != nil {
return err
}
sdkHelper := ackmodel.NewSDKHelper(sdkDir)
sdkAPI, err := sdkHelper.API(svcAlias)
if err != nil {
newSvcAlias, err := FallBackFindServiceID(sdkDir, svcAlias)
if err != nil {
return err
}
sdkAPI, err = sdkHelper.API(newSvcAlias) // retry with serviceID
if err != nil {
return fmt.Errorf("service %s not found", svcAlias)
}
}

latestAPIVersion, err = getLatestAPIVersion()
if err != nil {
return err
}
m, err := ackmodel.New(
sdkAPI, latestAPIVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
m, err := loadModelWithLatestAPIVersion(svcAlias)
if err != nil {
return err
}
Expand Down
17 changes: 1 addition & 16 deletions cmd/ack-generate/command/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

ackgenerate "github.com/aws-controllers-k8s/code-generator/pkg/generate/ack"
ackmetadata "github.com/aws-controllers-k8s/code-generator/pkg/metadata"
ackmodel "github.com/aws-controllers-k8s/code-generator/pkg/model"
)

var (
Expand Down Expand Up @@ -74,21 +73,7 @@ func generateRelease(cmd *cobra.Command, args []string) error {
if err := ensureSDKRepo(ctx, optCacheDir, optRefreshCache); err != nil {
return err
}
sdkHelper := ackmodel.NewSDKHelper(sdkDir)
sdkAPI, err := sdkHelper.API(svcAlias)
if err != nil {
newSvcAlias, err := FallBackFindServiceID(sdkDir, svcAlias)
if err != nil {
return err
}
sdkAPI, err = sdkHelper.API(newSvcAlias) // retry with serviceID
if err != nil {
return fmt.Errorf("service %s not found", svcAlias)
}
}
m, err := ackmodel.New(
sdkAPI, "", optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
m, err := loadModel(svcAlias, "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could you add a comment as to why the apiVersion is empty here? If it doesn't matter, then loadModelWithLatestAPIVersion would be clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually couldn't tell you why apiVersion needs to be empty string, only for release. I get an abstract error when I provide the latest API version. I believe this may because of the multiversion support - you would not need to specify an API version since it assumes each of them individually to create the webhook.

Copy link
Contributor Author

@RedbackThomson RedbackThomson Oct 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a-hilaly I would love your input on this.

if err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/generate/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ type Config struct {
// SetManyOutput function fails with NotFound error.
// Default is "return nil, ackerr.NotFound"
SetManyOutputNotFoundErrReturn string `json:"set_many_output_notfound_err_return,omitempty"`
// ModelName lets you specify the path used to identify the AWS service API
RedbackThomson marked this conversation as resolved.
Show resolved Hide resolved
// in the aws-sdk-go's models/apis/ directory. This field is optional and
// only needed for services such as the opensearchservice service where the
// model name is `opensearch` and the service package is called
// `opensearchservice`.
ModelName string `json:"model_name,omitempty"`
RedbackThomson marked this conversation as resolved.
Show resolved Hide resolved
}

// IgnoreSpec represents instructions to the ACK code generator to
Expand Down
8 changes: 4 additions & 4 deletions pkg/generate/crossplane/crossplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func Crossplane(
for _, path := range apisGenericTemplatesPaths {
outPath := filepath.Join(
"apis",
metaVars.ServiceIDClean,
metaVars.ServiceAlias,
metaVars.APIVersion,
"zz_"+strings.TrimSuffix(filepath.Base(path), ".tpl"),
)
Expand All @@ -155,7 +155,7 @@ func Crossplane(
}
for _, crd := range crds {
crdFileName := filepath.Join(
"apis", metaVars.ServiceIDClean, metaVars.APIVersion,
"apis", metaVars.ServiceAlias, metaVars.APIVersion,
"zz_"+strcase.ToSnake(crd.Kind)+".go",
)
crdVars := &templateCRDVars{
Expand All @@ -170,7 +170,7 @@ func Crossplane(
// Next add the controller package for each CRD
for _, crd := range crds {
outPath := filepath.Join(
"pkg", "controller", metaVars.ServiceIDClean, crd.Names.Lower,
"pkg", "controller", metaVars.ServiceAlias, crd.Names.Lower,
"zz_controller.go",
)
crdVars := &templateCRDVars{
Expand All @@ -181,7 +181,7 @@ func Crossplane(
return nil, err
}
outPath = filepath.Join(
"pkg", "controller", metaVars.ServiceIDClean, crd.Names.Lower,
"pkg", "controller", metaVars.ServiceAlias, crd.Names.Lower,
"zz_conversions.go",
)
crdVars = &templateCRDVars{
Expand Down
2 changes: 1 addition & 1 deletion pkg/generate/olm/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func BundleAssets(

csvBaseOutPath := fmt.Sprintf(
"config/manifests/bases/ack-%s-controller.clusterserviceversion.yaml",
m.MetaVars().ServiceIDClean)
m.MetaVars().ServiceAlias)
if err := ts.Add(csvBaseOutPath, "config/manifests/bases/clusterserviceversion.yaml.tpl", olmVars); err != nil {
return nil, err
}
Expand Down
13 changes: 7 additions & 6 deletions pkg/generate/templateset/vars.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@ package templateset
// that describe the service alias, its package name, etc
type MetaVars struct {
// ServiceAlias contains the exact string used to identify the AWS service
// API in the aws-sdk-go's models/apis/ directory. Note that some APIs this
// alias does not match the ServiceID. e.g. The AWS Step Functions API has
// a ServiceID of "SFN" and a service alias of "states"...
// API in the aws-sdk-go `service/` directory. It is also used as the
// identifier for the ACK controller's name and packages.
ServiceAlias string
// ServiceID is the exact string that appears in the AWS service API's
// api-2.json descriptor file under `metadata.serviceId`
ServiceID string
// ServiceIDClean is the ServiceID lowercased and stripped of any
// non-alphanumeric characters
ServiceIDClean string
// ServiceModelName contains the exact string used to identify the AWS
// service API in the aws-sdk-go's models/apis/ directory. Note that some
// APIs this name does not match the ServiceID. e.g. The AWS Step Functions
// API has a ServiceID of "SFN" and a service model name of "states"...
ServiceModelName string
// APIVersion contains the version of the Kubernetes API resources, e.g.
// "v1alpha1"
APIVersion string
Expand Down
Loading