Introduce `ack-generate webhooks` sub command #122

a-hilaly · 2021-07-06T14:56:02Z

Part of aws-controllers-k8s/community#835

This patch brings in a new sub command that can be used to generate
webhook functions. For now it can only generate conversion webhook
functions, but it can be extended to generate defaulting and validating
functions.

This patch also introduces a new function that generates conversion
functions. It can be used to generate ConvertTo and ConvertFrom
functions.

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

ack-bot · 2021-07-06T14:56:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly

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]

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

a-hilaly · 2021-07-06T14:56:56Z

NOTE to reviews: This will need a rebase after #120 and #121 merge.
/hold

RedbackThomson · 2021-07-12T23:44:01Z

Could we hold off on this command until after I propose the metadata configuration? Options such as deprecated versions, etc. will all be available in that configuration file and it will not be necessary to pass them as command line arguments.

a-hilaly · 2021-07-13T08:42:02Z

Could we hold off on this command until after I propose the metadata configuration? Options such as deprecated versions, etc. will all be available in that configuration file and it will not be necessary to pass them as command line arguments.

Sure. I will rebase on top of your work once it's available.

a-hilaly · 2021-07-13T15:06:08Z

pkg/generate/ack/output.go

+// LoadGenerationMetadata read the generation metadata for a given api version and
+// apis path.
+func LoadGenerationMetadata(apisPath, apiVersion string) (*GenerationMetadata, error) {
+	filePath := filepath.Join(apisPath, apiVersion, outputFileName)
+	b, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return nil, err
+	}
+
+	var generationMetadata GenerationMetadata
+	err = yaml.Unmarshal(b, &generationMetadata)
+	if err != nil {
+		return nil, err
+	}
+	return &generationMetadata, nil
+}
+


@RedbackThomson If you can implement a similar function in your PR, i'm taking it :)

These have been moved to pkg/metadata

a-hilaly · 2021-07-16T13:50:17Z

Need to be rebased, now that #126 is merged
/hold

jaypipes

@a-hilaly wonderful start on this! You will need to make a number of changes, however, in order to properly deal with nested complex types. See inline for an explanation of why your variable-naming schema needs to be adapted.

cmd/ack-generate/command/webhook.go

pkg/generate/ack/webhook.go

pkg/generate/code/converter/converter.go

jaypipes · 2021-07-16T14:22:47Z

pkg/generate/code/converter/converter.go

+	// elementCopy := make([]*v1alpha2.WebhookFilter, 0, len(element))
+	varElementCopy := "element" + "Copy"


You are going to run into problems with nested list types here, because you will have variable name shadowing in effect. This is why the code generation that sets CR fields from an Output shape (the pkg/generate/code.SetOutput stuff) generates a variable name that represents the field's numeric index within the Shape. For instance, if I had a Shape like this:

type Bar struct { Baz string } type Foo struct { Bars []*Bar } type MyShape struct { Foos []*Foo }

The variable referring to MyShape.Foos.Bars would be called f0f0 and the iterator over elements in that list of Bar structs would be called f0f0iter, with the element variable name of f0f0elem.

The way you are naming things here means that you would end up producing Go code like this:

elementCopy := make([]*Foo, 0, len(src.Foos)) for _, element := range src.Foos { fooCopy := &Foo{} elementCopy := make([]*Bar, 0, len(element.Bars) for _, element := range element.Bars { barCopy := &Bar{} barCopy.Baz = element.Baz elementCopy = append(elementCopy, barCopy) } elementCopy = append(elementCopy, fooCopy) }

which would be invalid since the variable name elementCopy is shadowed/hidden by the inner declaration of itself.

jaypipes · 2021-07-16T14:24:55Z

pkg/generate/code/converter/converter.go

+	)
+
+	memberShape := shape.ValueRef.Shape
+	varElementCopy := "element" + "Copy"


same comment as above :) you are going to run into variable naming conflicts.

jaypipes · 2021-07-16T14:25:59Z

pkg/generate/code/converter/converter.go

+	return out
+}
+
+// storeVariableIn retruns go code that stores a value in a given variable.


s/retruns go/returns Go

functions This patch introduces a new function that generates conversion functions. It can be used to generate `ConvertTo` and `ConvertFrom` functions.

This patch brings in a new sub command that can be used to generate webhook functions. For now it can only generate conversion webhook functions, but it can be used to generate defaulting and validating functions. This patch also introduces a new function to load existing ack-generate metadata for a given api version.

…s not empty

a-hilaly · 2021-08-24T09:02:52Z

/do-not-merge needs-rebase

ack-bot · 2021-11-22T11:39:11Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

ack-bot · 2021-12-22T11:39:35Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle rotten

a-hilaly · 2021-12-22T11:41:09Z

/remove-lifecycle rotten

ack-bot · 2022-03-22T17:40:49Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle stale

a-hilaly · 2022-03-22T17:53:20Z

/lifecycle frozen

ack-bot · 2022-03-22T18:07:15Z

@a-hilaly: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ack-bot · 2022-04-21T23:41:13Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/lifecycle rotten

ack-bot · 2022-05-22T05:13:47Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/close

ack-bot · 2022-05-22T05:13:49Z

@ack-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

a-hilaly · 2022-05-23T10:42:02Z

/reopen
/lifecycle frozen

ack-bot · 2022-05-23T10:42:05Z

@a-hilaly: Reopened this PR.

In response to this:

/reopen
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ack-bot · 2022-05-23T10:42:06Z

@a-hilaly: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/reopen
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ack-bot · 2022-05-23T10:47:15Z

@a-hilaly: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
s3-olm-test	`3a3f2f0`	link	`/test s3-olm-test`
dynamodb-controller-test	`3a3f2f0`	link	`/test dynamodb-controller-test`
s3-controller-test	`3a3f2f0`	link	`/test s3-controller-test`
ecr-controller-test	`3a3f2f0`	link	`/test ecr-controller-test`
unit-test	`3a3f2f0`	link	`/test unit-test`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ack-bot · 2022-06-22T11:14:14Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/close

ack-bot · 2022-06-22T11:14:17Z

@ack-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Provide feedback via https://github.com/aws-controllers-k8s/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ack-bot requested review from jaypipes and vijtrip2 July 6, 2021 14:56

ack-bot added the approved label Jul 6, 2021

ack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2021

a-hilaly force-pushed the ackgenerate-webhooks branch 3 times, most recently from 2d48795 to 2dcad55 Compare July 12, 2021 23:03

a-hilaly force-pushed the ackgenerate-webhooks branch 4 times, most recently from c149eb8 to 78c343c Compare July 13, 2021 15:01

a-hilaly commented Jul 13, 2021

View reviewed changes

a-hilaly force-pushed the ackgenerate-webhooks branch 3 times, most recently from d91e0ae to a36d4d1 Compare July 15, 2021 16:12

jaypipes requested changes Jul 16, 2021

View reviewed changes

a-hilaly added 3 commits July 22, 2021 17:25

Add pkg/generate/code/convert.go to help generating conversion

8cee9de

functions This patch introduces a new function that generates conversion functions. It can be used to generate `ConvertTo` and `ConvertFrom` functions.

prepend the replacePkgAlias in model.ReplacePkgName only when it'…

3a3f2f0

…s not empty

a-hilaly force-pushed the ackgenerate-webhooks branch 2 times, most recently from 282df9a to 3a3f2f0 Compare August 24, 2021 08:56

ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 22, 2021

a-hilaly mentioned this pull request Dec 8, 2021

Ability to provide API version as resource-specific configuration aws-controllers-k8s/community#1089

Closed

ack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 22, 2021

ack-bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Dec 22, 2021

a-hilaly mentioned this pull request Jan 28, 2022

Make CRD version configurable #266

Merged

ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 22, 2022

ack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 21, 2022

ack-bot closed this May 22, 2022

ack-bot reopened this May 23, 2022

ack-bot closed this Jun 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `ack-generate webhooks` sub command #122

Introduce `ack-generate webhooks` sub command #122

a-hilaly commented Jul 6, 2021

ack-bot commented Jul 6, 2021

a-hilaly commented Jul 6, 2021

RedbackThomson commented Jul 12, 2021

a-hilaly commented Jul 13, 2021

a-hilaly Jul 13, 2021

RedbackThomson Jul 16, 2021

a-hilaly commented Jul 16, 2021

jaypipes left a comment

jaypipes Jul 16, 2021

jaypipes Jul 16, 2021

jaypipes Jul 16, 2021

a-hilaly commented Aug 24, 2021

ack-bot commented Nov 22, 2021

ack-bot commented Dec 22, 2021

a-hilaly commented Dec 22, 2021

ack-bot commented Mar 22, 2022

a-hilaly commented Mar 22, 2022

ack-bot commented Mar 22, 2022

ack-bot commented Apr 21, 2022

ack-bot commented May 22, 2022

ack-bot commented May 22, 2022

a-hilaly commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented Jun 22, 2022

ack-bot commented Jun 22, 2022

		// elementCopy := make([]*v1alpha2.WebhookFilter, 0, len(element))
		varElementCopy := "element" + "Copy"

Introduce ack-generate webhooks sub command #122

Introduce ack-generate webhooks sub command #122

Conversation

a-hilaly commented Jul 6, 2021

ack-bot commented Jul 6, 2021

a-hilaly commented Jul 6, 2021

RedbackThomson commented Jul 12, 2021

a-hilaly commented Jul 13, 2021

a-hilaly Jul 13, 2021

Choose a reason for hiding this comment

RedbackThomson Jul 16, 2021

Choose a reason for hiding this comment

a-hilaly commented Jul 16, 2021

jaypipes left a comment

Choose a reason for hiding this comment

jaypipes Jul 16, 2021

Choose a reason for hiding this comment

jaypipes Jul 16, 2021

Choose a reason for hiding this comment

jaypipes Jul 16, 2021

Choose a reason for hiding this comment

a-hilaly commented Aug 24, 2021

ack-bot commented Nov 22, 2021

ack-bot commented Dec 22, 2021

a-hilaly commented Dec 22, 2021

ack-bot commented Mar 22, 2022

a-hilaly commented Mar 22, 2022

ack-bot commented Mar 22, 2022

ack-bot commented Apr 21, 2022

ack-bot commented May 22, 2022

ack-bot commented May 22, 2022

a-hilaly commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented May 23, 2022

ack-bot commented Jun 22, 2022

ack-bot commented Jun 22, 2022

Introduce `ack-generate webhooks` sub command #122

Introduce `ack-generate webhooks` sub command #122