Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
7 changes: 4 additions & 3 deletions cmd/ack-generate/command/apis.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/spf13/cobra"

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 @@ -59,10 +60,10 @@ func init() {
// saveGeneratedMetadata saves the parameters used to generate APIs and checksum
// of the generated code.
func saveGeneratedMetadata(cmd *cobra.Command, args []string) error {
err := ackgenerate.CreateGenerationMetadata(
err := ackmetadata.CreateGenerationMetadata(
optGenVersion,
filepath.Join(optOutputPath, "apis"),
ackgenerate.UpdateReasonAPIGeneration,
ackmetadata.UpdateReasonAPIGeneration,
optAWSSDKGoVersion,
optGeneratorConfigPath,
)
Expand Down Expand Up @@ -109,7 +110,7 @@ func generateAPIs(cmd *cobra.Command, args []string) error {
}
}
model, err := ackmodel.New(
sdkAPI, optGenVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
sdkAPI, optGenVersion, optMetadataConfigPath, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/ack-generate/command/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func generateController(cmd *cobra.Command, args []string) error {
return err
}
m, err := ackmodel.New(
sdkAPI, latestAPIVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
sdkAPI, latestAPIVersion, optMetadataConfigPath, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/ack-generate/command/crossplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func generateCrossplane(_ *cobra.Command, args []string) error {
cfgPath = ""
}
m, err := ackmodel.New(
sdkAPI, optGenVersion, cfgPath, cpgenerate.DefaultConfig,
sdkAPI, optGenVersion, optMetadataConfigPath, cfgPath, cpgenerate.DefaultConfig,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/ack-generate/command/olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func generateOLMAssets(cmd *cobra.Command, args []string) error {
return err
}
m, err := ackmodel.New(
sdkAPI, latestAPIVersion, optGeneratorConfigPath, ackgenerate.DefaultConfig,
sdkAPI, latestAPIVersion, optMetadataConfigPath, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cmd/ack-generate/command/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func generateRelease(cmd *cobra.Command, args []string) error {
}
}
m, err := ackmodel.New(
sdkAPI, "", optGeneratorConfigPath, ackgenerate.DefaultConfig,
sdkAPI, "", optMetadataConfigPath, optGeneratorConfigPath, ackgenerate.DefaultConfig,
)
if err != nil {
return err
Expand Down
4 changes: 4 additions & 0 deletions cmd/ack-generate/command/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ var (
optDryRun bool
sdkDir string
optGeneratorConfigPath string
optMetadataConfigPath string
optOutputPath string
)

Expand Down Expand Up @@ -111,6 +112,9 @@ func init() {
rootCmd.PersistentFlags().StringVar(
&optGeneratorConfigPath, "generator-config-path", "", "Path to file containing instructions for code generation to use",
)
rootCmd.PersistentFlags().StringVar(
&optMetadataConfigPath, "metadata-config-path", "", "Path to file containing service metadata to use",
)
rootCmd.PersistentFlags().StringVarP(
&optOutputPath, "output", "o", "", "Path to directory to output generated files.",
)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/spf13/cobra v1.1.1
github.com/stretchr/testify v1.7.0
golang.org/x/mod v0.4.1
golang.org/x/tools v0.0.0-20200616195046-dc31b401abb5
Copy link
Collaborator

Choose a reason for hiding this comment

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

where'd this come from? maybe remove it with go mod tidy?

gopkg.in/src-d/go-git.v4 v4.13.1
k8s.io/apimachinery v0.20.1
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/aws-controllers-k8s/runtime v0.4.0 h1:qsCp1OKXoSMtjS+6zlH65PN3/UkVJNbI/ybwYjPmhrY=
github.com/aws-controllers-k8s/runtime v0.4.0/go.mod h1:xA2F18PJerBHaqrS4de1lpP7skeSMeStkmh+3x5sWvw=
github.com/aws-controllers-k8s/runtime v0.5.0 h1:lV2QxuEHHKS+nbJ7lYgPjwVBY6Aqw+RHJF8eDZZeswM=
github.com/aws-controllers-k8s/runtime v0.5.0/go.mod h1:xA2F18PJerBHaqrS4de1lpP7skeSMeStkmh+3x5sWvw=
github.com/aws/aws-sdk-go v1.37.4 h1:tWxrpMK/oRSXVnjUzhGeCWLR00fW0WF4V4sycYPPrJ8=
github.com/aws/aws-sdk-go v1.37.4/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
Expand Down
15 changes: 6 additions & 9 deletions pkg/model/multiversion/api_info.go → pkg/metadata/api_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,16 @@
// express or implied. See the License for the specific language governing
// permissions and limitations under the License.

package multiversion

// TODO(a-hilaly) move this file outside of pkg/model/multiversion. Idealy we
// Should be able to access APIStatus and APIInfo to prevent regenerating removed or
// deprecated APIs.
package metadata

type APIStatus string

const (
APIStatusUnknown APIStatus = "unknown"
APIStatusAvailable = "available"
APIStatusRemoved = "removed"
APIStatusDeprecated = "deprecated"
APIStatusUnknown APIStatus = "unknown"
APIStatusInDevelopment = "in_development"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we call this just development and make it the default instead of unknown?

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we really need a development status for API versions. I think that we should increase the API Version whenever the CRD schema changes.

Copy link
Member

@a-hilaly a-hilaly Jul 14, 2021

Choose a reason for hiding this comment

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

IMO development should be a status for the service controller and not a specific API version. Unless, if development means "not released" with the controller helm chart yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That seems fair. The API version in development will always be the latest available option. "In development" vs. "released" is controlled by our controller release already.

APIStatusAvailable = "available"
APIStatusRemoved = "removed"
APIStatusDeprecated = "deprecated"
)

// APIInfo contains information related a specific apiVersion.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// express or implied. See the License for the specific language governing
// permissions and limitations under the License.

package ack
package metadata

import (
"crypto/sha1"
Expand Down
118 changes: 118 additions & 0 deletions pkg/metadata/service_metadata.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
// License is located at
//
// http://aws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file is distributed
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
// express or implied. See the License for the specific language governing
// permissions and limitations under the License.

package metadata

import (
"fmt"
"io/ioutil"

"github.com/ghodss/yaml"
)

// ServiceMetadata consists of information about the service and relative links as well
// as a list of supported/deprecated versions
type ServiceMetadata struct {
Service ServiceDetails `json:"service"`
// CRDs to ignore. ACK generator would skip these resources.
Versions []ServiceVersion `json:"versions"`
}

// ServiceDetails contains string identifiers and relevant links for the
// service
type ServiceDetails struct {
// The full display name for the service. eg. Amazon Elastic Kubernetes
// Service
FullName string `json:"full_name"`
// The short name (abbreviation) for the service. eg. S3
ShortName string `json:"short_name"`
// The URL of the service's homepage
Link string `json:"link"`
// The URL of the service's main documentation/user guide
Documentation string `json:"documentation"`
}

// ServiceVersion describes the status of all existing version of the controller
type ServiceVersion struct {
APIVersion string `json:"api_version"`
Status APIStatus `json:"status"`
}
Comment on lines +51 to +54
Copy link
Member

Choose a reason for hiding this comment

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

We could also add a boolean field called IsHub, to be able to override the hub version. In most of the cases the latest Available version is the hub (prefered storage version). However, according to k8s deprecation policy sometimes the hub can be different from the latest version.

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 am quite averse to the idea of the hub being anything other than the latest generated version - we rely on it being the latest for the common runtime elements to match.


func (m *ServiceMetadata) GetLatestAPIVersion() (string, error) {
availableVersions := m.GetAvailableAPIVersions()

if len(availableVersions) == 0 {
return "", fmt.Errorf("service metadata contains no available versions")
}

return availableVersions[len(availableVersions)-1], nil
}

func (m *ServiceMetadata) GetDeprecatedAPIVersions() []string {
return m.getVersionsByStatus(APIStatusDeprecated)
}

func (m *ServiceMetadata) GetRemoveAPIVersions() []string {
return m.getVersionsByStatus(APIStatusRemoved)
}

func (m *ServiceMetadata) GetAvailableAPIVersions() []string {
return m.getVersionsByStatus(APIStatusAvailable)
}

func (m *ServiceMetadata) GetDevelopmentAPIVersion() (string, error) {
devVersions := m.getVersionsByStatus(APIStatusInDevelopment)

if len(devVersions) == 0 {
return "", fmt.Errorf("service metadata contains no development versions")
}

if len(devVersions) > 1 {
return "", fmt.Errorf("service metadata contains multiple development versions")
}

return devVersions[0], nil
}

func (m *ServiceMetadata) getVersionsByStatus(status APIStatus) []string {
if len(m.Versions) == 0 {
return []string{}
}

versions := []string{}
for _, v := range m.Versions {
if v.Status == status {
versions = append(versions, v.APIVersion)
}
}
return versions
}

// NewServiceMetadata returns a new Metadata object given a supplied
// path to a metadata file
func NewServiceMetadata(
metadataPath string,
) (ServiceMetadata, error) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can return a *ServiceMetadata to avoid allocating a new ServiceMetadata{} when you return an error

if metadataPath == "" {
return ServiceMetadata{}, fmt.Errorf("expected metadata file path, none provided")
}
content, err := ioutil.ReadFile(metadataPath)
if err != nil {
return ServiceMetadata{}, err
}
gc := ServiceMetadata{}
if err = yaml.Unmarshal(content, &gc); err != nil {
return ServiceMetadata{}, err
}
return gc, nil
}
10 changes: 10 additions & 0 deletions pkg/model/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

ackgenconfig "github.com/aws-controllers-k8s/code-generator/pkg/generate/config"
"github.com/aws-controllers-k8s/code-generator/pkg/generate/templateset"
ackmetadata "github.com/aws-controllers-k8s/code-generator/pkg/metadata"
"github.com/aws-controllers-k8s/code-generator/pkg/names"
"github.com/aws-controllers-k8s/code-generator/pkg/util"
)
Expand All @@ -40,6 +41,8 @@ type Model struct {
typeDefs []*TypeDef
typeImports map[string]string
typeRenames map[string]string
// Metadata for the service
meta *ackmetadata.ServiceMetadata
Copy link
Member

@a-hilaly a-hilaly Jul 14, 2021

Choose a reason for hiding this comment

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

Do we need *ackmetadata.ServiceMetadata in ackmode.Model and multiversion.APIVersionManager? what are the use cases when generating one single api version (ack-generate apis)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I had this in before your last PR, I'll remove it again.

// Instructions to the code generator how to handle the API and its
// resources
cfg *ackgenconfig.Config
Expand Down Expand Up @@ -699,9 +702,15 @@ func (m *Model) GetConfig() *ackgenconfig.Config {
func New(
SDKAPI *SDKAPI,
apiVersion string,
metadataPath string,
configPath string,
defaultConfig ackgenconfig.Config,
) (*Model, error) {
metadata, err := ackmetadata.NewServiceMetadata(metadataPath)
if err != nil {
return nil, err
}

cfg, err := ackgenconfig.New(configPath, defaultConfig)
if err != nil {
return nil, err
Expand All @@ -713,6 +722,7 @@ func New(
serviceAlias: SDKAPI.ServiceID(),
apiVersion: apiVersion,
cfg: &cfg,
meta: &metadata,
}
m.ApplyShapeIgnoreRules()
return m, nil
Expand Down
Loading