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

fix CRD Go struct generation for SNS and Lambda #385

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

jaypipes
Copy link
Collaborator

#383 fixed generation of CRD Go structs for SNS and SQS. Unfortunately, that PR broke Lambda, which has some fields that do not have ShapeRef attributes.

aws-controllers-k8s/code-generato#384 fixed Lambda but subsequently re-broke SNS and SQS generation, with make build-controller SERVICE=sns producing erroneous whitespace-different CRD Go structs:

[jaypipes@thelio sns-controller]$ git diff
diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml
index 81fbab0..a894215 100755
--- a/apis/v1alpha1/ack-generate-metadata.yaml
+++ b/apis/v1alpha1/ack-generate-metadata.yaml
@@ -1,9 +1,9 @@
 ack_generate_info:
-  build_date: "2022-12-16T20:24:15Z"
-  build_hash: e661ce95afc39b380653ca655503daebf1e1831b
+  build_date: "2022-12-19T18:13:35Z"
+  build_hash: 8d843f48df4504e3d19712c0e27dbb388ff40cc0
   go_version: go1.18.1
-  version: v0.21.0-5-ge661ce9
-api_directory_checksum: a0c05230761bda067a0f03b794041014296b8bcb
+  version: v0.21.0-6-g8d843f4
+api_directory_checksum: b10135ae96fc0459ddaa92c9ea9429424b052424
 api_version: v1alpha1
 aws_sdk_go_version: v1.44.93
 generator_config_info:
diff --git a/apis/v1alpha1/platform_application.go b/apis/v1alpha1/platform_application.go
index e0402de..9e757fe 100644
--- a/apis/v1alpha1/platform_application.go
+++ b/apis/v1alpha1/platform_application.go
@@ -24,10 +24,14 @@ import (
 //
 // Platform application object.
 type PlatformApplicationSpec struct {
-       EventDeliveryFailure   *string `json:"eventDeliveryFailure,omitempty"`
-       EventEndpointCreated   *string `json:"eventEndpointCreated,omitempty"`
-       EventEndpointDeleted   *string `json:"eventEndpointDeleted,omitempty"`
-       EventEndpointUpdated   *string `json:"eventEndpointUpdated,omitempty"`
+       EventDeliveryFailure *string `json:"eventDeliveryFailure,omitempty"`
+
+       EventEndpointCreated *string `json:"eventEndpointCreated,omitempty"`
+
+       EventEndpointDeleted *string `json:"eventEndpointDeleted,omitempty"`
+
+       EventEndpointUpdated *string `json:"eventEndpointUpdated,omitempty"`
+
        FailureFeedbackRoleARN *string `json:"failureFeedbackRoleARN,omitempty"`
        // Application names must be made up of only uppercase and lowercase ASCII letters,
        // numbers, underscores, hyphens, and periods, and must be between 1 and 256
@@ -38,10 +42,14 @@ type PlatformApplicationSpec struct {
        // (Apple Push Notification Service), APNS_SANDBOX, and GCM (Firebase Cloud
        // Messaging).
        // +kubebuilder:validation:Required
-       Platform                  *string `json:"platform"`
-       PlatformCredential        *string `json:"platformCredential,omitempty"`
-       PlatformPrincipal         *string `json:"platformPrincipal,omitempty"`
-       SuccessFeedbackRoleARN    *string `json:"successFeedbackRoleARN,omitempty"`
+       Platform *string `json:"platform"`
+
+       PlatformCredential *string `json:"platformCredential,omitempty"`
+
+       PlatformPrincipal *string `json:"platformPrincipal,omitempty"`
+
+       SuccessFeedbackRoleARN *string `json:"successFeedbackRoleARN,omitempty"`
+
        SuccessFeedbackSampleRate *string `json:"successFeedbackSampleRate,omitempty"`
 }

diff --git a/apis/v1alpha1/platform_endpoint.go b/apis/v1alpha1/platform_endpoint.go
index e24fd42..b809a73 100644
--- a/apis/v1alpha1/platform_endpoint.go
+++ b/apis/v1alpha1/platform_endpoint.go
@@ -23,12 +23,14 @@ import (
 // PlatformEndpointSpec defines the desired state of PlatformEndpoint.
 type PlatformEndpointSpec struct {
        CustomUserData *string `json:"customUserData,omitempty"`
-       Enabled        *string `json:"enabled,omitempty"`
+
+       Enabled *string `json:"enabled,omitempty"`
        // PlatformApplicationArn returned from CreatePlatformApplication is used to
        // create a an endpoint.
        // +kubebuilder:validation:Required
        PlatformApplicationARN *string `json:"platformApplicationARN"`
-       Token                  *string `json:"token,omitempty"`
+
+       Token *string `json:"token,omitempty"`
 }

 // PlatformEndpointStatus defines the observed state of PlatformEndpoint
diff --git a/apis/v1alpha1/topic.go b/apis/v1alpha1/topic.go
index 31d1506..87b35f7 100644
--- a/apis/v1alpha1/topic.go
+++ b/apis/v1alpha1/topic.go
@@ -34,9 +34,12 @@ type TopicSpec struct {
        //
        // Length Constraints: Maximum length of 30,720.
        DataProtectionPolicy *string `json:"dataProtectionPolicy,omitempty"`
-       DeliveryPolicy       *string `json:"deliveryPolicy,omitempty"`
-       DisplayName          *string `json:"displayName,omitempty"`
-       KMSMasterKeyID       *string `json:"kmsMasterKeyID,omitempty"`
+
+       DeliveryPolicy *string `json:"deliveryPolicy,omitempty"`
+
+       DisplayName *string `json:"displayName,omitempty"`
+
+       KMSMasterKeyID *string `json:"kmsMasterKeyID,omitempty"`
        // The name of the topic you want to create.
        //
        // Constraints: Topic names must be made up of only uppercase and lowercase
@@ -45,7 +48,8 @@ type TopicSpec struct {
        //
        // For a FIFO (first-in-first-out) topic, the name must end with the .fifo suffix.
        // +kubebuilder:validation:Required
-       Name   *string `json:"name"`
+       Name *string `json:"name"`
+
        Policy *string `json:"policy,omitempty"`

This patch fixes the issue once and for all by checking both $field.ShapeRef for existence as well as
$field.ShapeRef.Documentation for a non-zero length.

Issue aws-controllers-k8s/community#1600

Signed-off-by: Jay Pipes jaypipes@gmail.com

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

aws-controllers-k8s#383 fixed generation of CRD Go structs for SNS and SQS. Unfortunately, that PR broke Lambda, which has some fields that do not have `ShapeRef` attributes.

aws-controllers-k8s/code-generato#384 fixed Lambda but subsequently re-broke SNS and SQS generation, with `make build-controller SERVICE=sns` producing erroneous whitespace-different CRD Go structs:

```diff
[jaypipes@thelio sns-controller]$ git diff
diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml
index 81fbab0..a894215 100755
--- a/apis/v1alpha1/ack-generate-metadata.yaml
+++ b/apis/v1alpha1/ack-generate-metadata.yaml
@@ -1,9 +1,9 @@
 ack_generate_info:
-  build_date: "2022-12-16T20:24:15Z"
-  build_hash: e661ce9
+  build_date: "2022-12-19T18:13:35Z"
+  build_hash: 8d843f4
   go_version: go1.18.1
-  version: v0.21.0-5-ge661ce9
-api_directory_checksum: a0c05230761bda067a0f03b794041014296b8bcb
+  version: v0.21.0-6-g8d843f4
+api_directory_checksum: b10135ae96fc0459ddaa92c9ea9429424b052424
 api_version: v1alpha1
 aws_sdk_go_version: v1.44.93
 generator_config_info:
diff --git a/apis/v1alpha1/platform_application.go b/apis/v1alpha1/platform_application.go
index e0402de..9e757fe 100644
--- a/apis/v1alpha1/platform_application.go
+++ b/apis/v1alpha1/platform_application.go
@@ -24,10 +24,14 @@ import (
 //
 // Platform application object.
 type PlatformApplicationSpec struct {
-       EventDeliveryFailure   *string `json:"eventDeliveryFailure,omitempty"`
-       EventEndpointCreated   *string `json:"eventEndpointCreated,omitempty"`
-       EventEndpointDeleted   *string `json:"eventEndpointDeleted,omitempty"`
-       EventEndpointUpdated   *string `json:"eventEndpointUpdated,omitempty"`
+       EventDeliveryFailure *string `json:"eventDeliveryFailure,omitempty"`
+
+       EventEndpointCreated *string `json:"eventEndpointCreated,omitempty"`
+
+       EventEndpointDeleted *string `json:"eventEndpointDeleted,omitempty"`
+
+       EventEndpointUpdated *string `json:"eventEndpointUpdated,omitempty"`
+
        FailureFeedbackRoleARN *string `json:"failureFeedbackRoleARN,omitempty"`
        // Application names must be made up of only uppercase and lowercase ASCII letters,
        // numbers, underscores, hyphens, and periods, and must be between 1 and 256
@@ -38,10 +42,14 @@ type PlatformApplicationSpec struct {
        // (Apple Push Notification Service), APNS_SANDBOX, and GCM (Firebase Cloud
        // Messaging).
        // +kubebuilder:validation:Required
-       Platform                  *string `json:"platform"`
-       PlatformCredential        *string `json:"platformCredential,omitempty"`
-       PlatformPrincipal         *string `json:"platformPrincipal,omitempty"`
-       SuccessFeedbackRoleARN    *string `json:"successFeedbackRoleARN,omitempty"`
+       Platform *string `json:"platform"`
+
+       PlatformCredential *string `json:"platformCredential,omitempty"`
+
+       PlatformPrincipal *string `json:"platformPrincipal,omitempty"`
+
+       SuccessFeedbackRoleARN *string `json:"successFeedbackRoleARN,omitempty"`
+
        SuccessFeedbackSampleRate *string `json:"successFeedbackSampleRate,omitempty"`
 }

diff --git a/apis/v1alpha1/platform_endpoint.go b/apis/v1alpha1/platform_endpoint.go
index e24fd42..b809a73 100644
--- a/apis/v1alpha1/platform_endpoint.go
+++ b/apis/v1alpha1/platform_endpoint.go
@@ -23,12 +23,14 @@ import (
 // PlatformEndpointSpec defines the desired state of PlatformEndpoint.
 type PlatformEndpointSpec struct {
        CustomUserData *string `json:"customUserData,omitempty"`
-       Enabled        *string `json:"enabled,omitempty"`
+
+       Enabled *string `json:"enabled,omitempty"`
        // PlatformApplicationArn returned from CreatePlatformApplication is used to
        // create a an endpoint.
        // +kubebuilder:validation:Required
        PlatformApplicationARN *string `json:"platformApplicationARN"`
-       Token                  *string `json:"token,omitempty"`
+
+       Token *string `json:"token,omitempty"`
 }

 // PlatformEndpointStatus defines the observed state of PlatformEndpoint
diff --git a/apis/v1alpha1/topic.go b/apis/v1alpha1/topic.go
index 31d1506..87b35f7 100644
--- a/apis/v1alpha1/topic.go
+++ b/apis/v1alpha1/topic.go
@@ -34,9 +34,12 @@ type TopicSpec struct {
        //
        // Length Constraints: Maximum length of 30,720.
        DataProtectionPolicy *string `json:"dataProtectionPolicy,omitempty"`
-       DeliveryPolicy       *string `json:"deliveryPolicy,omitempty"`
-       DisplayName          *string `json:"displayName,omitempty"`
-       KMSMasterKeyID       *string `json:"kmsMasterKeyID,omitempty"`
+
+       DeliveryPolicy *string `json:"deliveryPolicy,omitempty"`
+
+       DisplayName *string `json:"displayName,omitempty"`
+
+       KMSMasterKeyID *string `json:"kmsMasterKeyID,omitempty"`
        // The name of the topic you want to create.
        //
        // Constraints: Topic names must be made up of only uppercase and lowercase
@@ -45,7 +48,8 @@ type TopicSpec struct {
        //
        // For a FIFO (first-in-first-out) topic, the name must end with the .fifo suffix.
        // +kubebuilder:validation:Required
-       Name   *string `json:"name"`
+       Name *string `json:"name"`
+
        Policy *string `json:"policy,omitempty"`
```

This patch fixes the issue once and for all by checking both
`$field.ShapeRef` for existence as well as
`$field.ShapeRef.Documentation` for a non-zero length.

Issue aws-controllers-k8s/community#1600

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

+++

Copy link
Contributor

@jljaco jljaco left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@jljaco
Copy link
Contributor

jljaco commented Dec 19, 2022

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 19, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Dec 19, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, jaypipes, jljaco

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [A-Hilaly,jaypipes,jljaco]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit 4e4052d into aws-controllers-k8s:main Dec 19, 2022
a-hilaly pushed a commit to a-hilaly/ack-code-generator that referenced this pull request Jan 5, 2023
)

aws-controllers-k8s#383 fixed generation of CRD Go structs for SNS and SQS. Unfortunately, that PR broke Lambda, which has some fields that do not have `ShapeRef` attributes.

aws-controllers-k8s/code-generato#384 fixed Lambda but subsequently re-broke SNS and SQS generation, with `make build-controller SERVICE=sns` producing erroneous whitespace-different CRD Go structs:

```diff
[jaypipes@thelio sns-controller]$ git diff
diff --git a/apis/v1alpha1/ack-generate-metadata.yaml b/apis/v1alpha1/ack-generate-metadata.yaml
index 81fbab0..a894215 100755
--- a/apis/v1alpha1/ack-generate-metadata.yaml
+++ b/apis/v1alpha1/ack-generate-metadata.yaml
@@ -1,9 +1,9 @@
 ack_generate_info:
-  build_date: "2022-12-16T20:24:15Z"
-  build_hash: e661ce9
+  build_date: "2022-12-19T18:13:35Z"
+  build_hash: 8d843f4
   go_version: go1.18.1
-  version: v0.21.0-5-ge661ce9
-api_directory_checksum: a0c05230761bda067a0f03b794041014296b8bcb
+  version: v0.21.0-6-g8d843f4
+api_directory_checksum: b10135ae96fc0459ddaa92c9ea9429424b052424
 api_version: v1alpha1
 aws_sdk_go_version: v1.44.93
 generator_config_info:
diff --git a/apis/v1alpha1/platform_application.go b/apis/v1alpha1/platform_application.go
index e0402de..9e757fe 100644
--- a/apis/v1alpha1/platform_application.go
+++ b/apis/v1alpha1/platform_application.go
@@ -24,10 +24,14 @@ import (
 //
 // Platform application object.
 type PlatformApplicationSpec struct {
-       EventDeliveryFailure   *string `json:"eventDeliveryFailure,omitempty"`
-       EventEndpointCreated   *string `json:"eventEndpointCreated,omitempty"`
-       EventEndpointDeleted   *string `json:"eventEndpointDeleted,omitempty"`
-       EventEndpointUpdated   *string `json:"eventEndpointUpdated,omitempty"`
+       EventDeliveryFailure *string `json:"eventDeliveryFailure,omitempty"`
+
+       EventEndpointCreated *string `json:"eventEndpointCreated,omitempty"`
+
+       EventEndpointDeleted *string `json:"eventEndpointDeleted,omitempty"`
+
+       EventEndpointUpdated *string `json:"eventEndpointUpdated,omitempty"`
+
        FailureFeedbackRoleARN *string `json:"failureFeedbackRoleARN,omitempty"`
        // Application names must be made up of only uppercase and lowercase ASCII letters,
        // numbers, underscores, hyphens, and periods, and must be between 1 and 256
@@ -38,10 +42,14 @@ type PlatformApplicationSpec struct {
        // (Apple Push Notification Service), APNS_SANDBOX, and GCM (Firebase Cloud
        // Messaging).
        // +kubebuilder:validation:Required
-       Platform                  *string `json:"platform"`
-       PlatformCredential        *string `json:"platformCredential,omitempty"`
-       PlatformPrincipal         *string `json:"platformPrincipal,omitempty"`
-       SuccessFeedbackRoleARN    *string `json:"successFeedbackRoleARN,omitempty"`
+       Platform *string `json:"platform"`
+
+       PlatformCredential *string `json:"platformCredential,omitempty"`
+
+       PlatformPrincipal *string `json:"platformPrincipal,omitempty"`
+
+       SuccessFeedbackRoleARN *string `json:"successFeedbackRoleARN,omitempty"`
+
        SuccessFeedbackSampleRate *string `json:"successFeedbackSampleRate,omitempty"`
 }

diff --git a/apis/v1alpha1/platform_endpoint.go b/apis/v1alpha1/platform_endpoint.go
index e24fd42..b809a73 100644
--- a/apis/v1alpha1/platform_endpoint.go
+++ b/apis/v1alpha1/platform_endpoint.go
@@ -23,12 +23,14 @@ import (
 // PlatformEndpointSpec defines the desired state of PlatformEndpoint.
 type PlatformEndpointSpec struct {
        CustomUserData *string `json:"customUserData,omitempty"`
-       Enabled        *string `json:"enabled,omitempty"`
+
+       Enabled *string `json:"enabled,omitempty"`
        // PlatformApplicationArn returned from CreatePlatformApplication is used to
        // create a an endpoint.
        // +kubebuilder:validation:Required
        PlatformApplicationARN *string `json:"platformApplicationARN"`
-       Token                  *string `json:"token,omitempty"`
+
+       Token *string `json:"token,omitempty"`
 }

 // PlatformEndpointStatus defines the observed state of PlatformEndpoint
diff --git a/apis/v1alpha1/topic.go b/apis/v1alpha1/topic.go
index 31d1506..87b35f7 100644
--- a/apis/v1alpha1/topic.go
+++ b/apis/v1alpha1/topic.go
@@ -34,9 +34,12 @@ type TopicSpec struct {
        //
        // Length Constraints: Maximum length of 30,720.
        DataProtectionPolicy *string `json:"dataProtectionPolicy,omitempty"`
-       DeliveryPolicy       *string `json:"deliveryPolicy,omitempty"`
-       DisplayName          *string `json:"displayName,omitempty"`
-       KMSMasterKeyID       *string `json:"kmsMasterKeyID,omitempty"`
+
+       DeliveryPolicy *string `json:"deliveryPolicy,omitempty"`
+
+       DisplayName *string `json:"displayName,omitempty"`
+
+       KMSMasterKeyID *string `json:"kmsMasterKeyID,omitempty"`
        // The name of the topic you want to create.
        //
        // Constraints: Topic names must be made up of only uppercase and lowercase
@@ -45,7 +48,8 @@ type TopicSpec struct {
        //
        // For a FIFO (first-in-first-out) topic, the name must end with the .fifo suffix.
        // +kubebuilder:validation:Required
-       Name   *string `json:"name"`
+       Name *string `json:"name"`
+
        Policy *string `json:"policy,omitempty"`
```

This patch fixes the issue once and for all by checking both `$field.ShapeRef` for existence as well as
`$field.ShapeRef.Documentation` for a non-zero length.

Issue aws-controllers-k8s/community#1600

Signed-off-by: Jay Pipes <jaypipes@gmail.com>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Amine Hilaly <hilalyamine@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants