⚠️ Marker Parsing and Structure #190

DirectXMan12 · 2019-04-16T06:00:13Z

This re-implements our parsing and generation logic in a more structured form.
I've tried to write up extended guides in the godoc for each new package, so check that
out until I put together a cohesive guide.

TODO:

a couple of documentation spots
design doc overview
better tests
iron out last few backwards-incompatibilities
support type-level schema modifications

DirectXMan12 · 2019-04-16T06:01:39Z

cc @mengqiy this has both the comment extraction and marker parsing.
There's a rough example in crd/v3/cmd/main.go

Don't accidentally check in ViM swap files, etc.

DirectXMan12 · 2019-04-27T06:06:40Z

@redborian FYI

droot · 2019-04-30T03:50:31Z

pkg/rbac/parser.go

+		return err
+	}
+
+	return nil
 }


RBAC Parser Reviewed. Looks good.

droot

Reviewing commit-by-commit.
Reviewed the marker collector/parsing, looks good, have couple of nits.

pkg/markers/doc.go

droot · 2019-04-30T18:19:37Z

pkg/markers/doc.go

+//
+// Markers can be collected from a loader.Package using a Collector.  The
+// Collector will read from a given Registry, collecting comments that look
+// like markers and parsing them if they match some definition on the registry.


This is the most important part which helped me understand how all these fit together.

pkg/markers/doc.go

droot · 2019-04-30T18:23:45Z

pkg/markers/doc.go

+// associated.
+//
+// PackageMarkers can be used to fetch just package-level markers.
+package markers


Excellent description.

pkg/markers/parse.go

droot

Reviewed genall and looks good to me.

pkg/genall/doc.go

pkg/genall/genall.go

droot · 2019-05-01T23:56:46Z

pkg/crd/spec.go

+	}
+
+	p.CustomResourceDefinitions[groupKind] = crd
+}


Reviewed crd parsing. This is bulk of the code I guess, looks pretty good on a high level.

droot

Reviewed the controller-gen changes, looks good overall.

cmd/controller-gen/main.go

droot

reviewed dep update, have one comment about controller-runtime deps.

go.sum

droot · 2019-05-02T00:12:18Z

Have reviewed all the changes (except deepcopy-gen) and looks good overall, have minor comments. You can probably submit deepcopy-gen in a separate PR if you would like to get the other changes in.

DirectXMan12 · 2019-05-02T06:04:13Z

Forgot webhook-gen. We'll need to make sure webhook gen works with our new plans before release -- I stripped out the parts I thought we no longer needed, but I just want to make sure.

DirectXMan12 · 2019-05-02T06:04:53Z

Added better tests for the low-level stuff. High-level stuff and schema operations need tests. We can follow up with those before release. Let's try and get this part merged tomorrow after I make sure all the ancillary tests pass (e.g. test project stuff), etc.

This introduces utilities for incremental/filtered package loading, based on go/packages (the new replacement for go/build and friends when importing). It'll work with modules properly, too.

droot

reviewed webhook generation commit, looks reasonable to me.

droot · 2019-05-03T17:30:06Z

pkg/webhook/parser.go

+//
+// The markers take the form:
+//
+//  +kubebuilder:rbac:failurePolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>


typo: s/rbac/webhook ?

s/rbac/webhook

@DirectXMan12 Please fix this.

droot

Reviewed testData import path commit, looks good.

droot

Looks good to me.

pkg/loader/loader.go

pkg/markers/collect.go

pkg/markers/doc.go

mengqiy · 2019-05-04T00:06:43Z

pkg/webhook/parser.go

+//
+// The markers take the form:
+//
+//  +kubebuilder:rbac:failurePolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>


s/rbac/webhook

@DirectXMan12 Please fix this.

mengqiy · 2019-05-04T00:07:41Z

pkg/webhook/parser.go

+//
+// The markers take the form:
+//
+//  +kubebuilder:rbac:failurePolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>


It seems it now requires all arguments to be in the same line. We need to remember to update the book.

This introduces a framework for collecting and parsing "marker" comments into structured Go data, much like the "json" package.

This removed the generation and parsing logic that's being superseded by loader, markers, etc.

This adds a generator framework that ties together running multiple generators from the same marker data, and describe how they can read and write artifacts.

This adds RBAC generation (as per our existing RBAC markers) on top of the new generator framework.

This defines the framework for CRD generation, and uses markers with that implement interfaces to modify the schema and CRD.

This implements deepcopy generation, mostly copying from deepcopy-gen but using the generator framework. It slightly scopes down things to only implement parts we care about for custom types that represent CRDs.

This updates controller-gen to make use of the new generator framework. Generators and ouput rules are treated as markers to parse commandline options.

This adds WebhookConfig generation (as per our existing webhook markers, with slight modifications) on top of the new generator framework.

It had an old import path pointing to the wrong place, which causes all sorts of build failures.

Update deps to required versions.

It's no longer used by any tests. We can always add it back in if we need to.

Some of the enabled checkers were deprecated, had known issues, or returned erroneous results. They've been commented out, with comments as to why they're not run.

DirectXMan12 · 2019-05-04T01:30:50Z

discussed with @mengqiy in person, will merge as per previous approval, then to master

DirectXMan12 · 2019-05-04T01:35:02Z

/lgtm
/approve

k8s-ci-robot · 2019-05-04T01:35:03Z

@DirectXMan12: you cannot LGTM your own PR.

In response to this:

/lgtm
/approve

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.

k8s-ci-robot · 2019-05-04T01:35:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12

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~~ [DirectXMan12]

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

mengqiy · 2019-05-04T01:38:07Z

/lgtm

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 16, 2019

k8s-ci-robot requested review from droot and pwittrock April 16, 2019 06:00

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 16, 2019

DirectXMan12 force-pushed the feature/marker-parsing branch 2 times, most recently from 133bc92 to 536a72d Compare April 24, 2019 06:17

Ignore ViM files

cb80f0b

Don't accidentally check in ViM swap files, etc.

DirectXMan12 force-pushed the feature/marker-parsing branch from 17c6907 to 502deab Compare April 27, 2019 06:03

This was referenced Apr 29, 2019

controller-gen crd: generate MetaData validation #195

Closed

Merge existing and newly generated CRD OpenAPIv3 validation blocks #156

Closed

droot reviewed Apr 30, 2019

View reviewed changes

pkg/rbac/parser.go

return err

}

return nil

}

Copy link

Contributor

droot Apr 30, 2019

Choose a reason for hiding this comment

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

RBAC Parser Reviewed. Looks good.

droot reviewed Apr 30, 2019

View reviewed changes

DirectXMan12 force-pushed the feature/marker-parsing branch from 502deab to 3518635 Compare May 1, 2019 05:45

droot reviewed May 1, 2019

View reviewed changes

pkg/genall/doc.go Outdated Show resolved Hide resolved

pkg/genall/genall.go Outdated Show resolved Hide resolved

droot reviewed May 1, 2019

View reviewed changes

droot reviewed May 2, 2019

View reviewed changes

cmd/controller-gen/main.go Show resolved Hide resolved

droot reviewed May 2, 2019

View reviewed changes

go.sum Outdated Show resolved Hide resolved

DirectXMan12 force-pushed the feature/marker-parsing branch 4 times, most recently from 00aaaa9 to 8a2a6fe Compare May 2, 2019 06:03

DirectXMan12 force-pushed the feature/marker-parsing branch from 8a2a6fe to e5fba3a Compare May 2, 2019 20:38

Package loading

f5cb7ce

This introduces utilities for incremental/filtered package loading, based on go/packages (the new replacement for go/build and friends when importing). It'll work with modules properly, too.

DirectXMan12 force-pushed the feature/marker-parsing branch 2 times, most recently from e6adb51 to 1a9e63e Compare May 3, 2019 06:33

droot reviewed May 3, 2019

View reviewed changes

mengqiy reviewed May 4, 2019

View reviewed changes

DirectXMan12 added 12 commits May 3, 2019 18:28

Marker parsing

f2ed5cb

This introduces a framework for collecting and parsing "marker" comments into structured Go data, much like the "json" package.

Remove old generation and parsing logic

1e02d93

This removed the generation and parsing logic that's being superseded by loader, markers, etc.

Generator framework

778eaea

This adds a generator framework that ties together running multiple generators from the same marker data, and describe how they can read and write artifacts.

RBAC Generation

6ec0406

This adds RBAC generation (as per our existing RBAC markers) on top of the new generator framework.

CRD generation

c264aec

This defines the framework for CRD generation, and uses markers with that implement interfaces to modify the schema and CRD.

Deepcopy generation

aec277d

This implements deepcopy generation, mostly copying from deepcopy-gen but using the generator framework. It slightly scopes down things to only implement parts we care about for custom types that represent CRDs.

New controller-gen

c989f19

This updates controller-gen to make use of the new generator framework. Generators and ouput rules are treated as markers to parse commandline options.

Webhook Generation

3a313d4

This adds WebhookConfig generation (as per our existing webhook markers, with slight modifications) on top of the new generator framework.

Fix testData import path

f2da9ab

It had an old import path pointing to the wrong place, which causes all sorts of build failures.

Update deps for new parsing

35b0aa2

Update deps to required versions.

Remove global testData

c363a49

It's no longer used by any tests. We can always add it back in if we need to.

Fix checker set

0fb1204

Some of the enabled checkers were deprecated, had known issues, or returned erroneous results. They've been commented out, with comments as to why they're not run.

DirectXMan12 force-pushed the feature/marker-parsing branch from 1a9e63e to 0fb1204 Compare May 4, 2019 01:30

DirectXMan12 changed the title ~~[WIP] ⚠️ Marker Parsing and Structure~~ ⚠️ Marker Parsing and Structure May 4, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 4, 2019

k8s-ci-robot assigned mengqiy May 4, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 4, 2019

k8s-ci-robot merged commit a22fb39 into kubernetes-sigs:crdgenerator May 4, 2019

DirectXMan12 deleted the feature/marker-parsing branch May 30, 2019 22:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ Marker Parsing and Structure #190

⚠️ Marker Parsing and Structure #190

DirectXMan12 commented Apr 16, 2019 •

edited

DirectXMan12 commented Apr 16, 2019 •

edited

DirectXMan12 commented Apr 27, 2019

droot Apr 30, 2019

droot left a comment

droot Apr 30, 2019

droot Apr 30, 2019

droot left a comment

droot May 1, 2019

droot left a comment

droot left a comment

droot commented May 2, 2019

DirectXMan12 commented May 2, 2019

DirectXMan12 commented May 2, 2019 •

edited

droot left a comment

droot May 3, 2019

mengqiy May 4, 2019 •

edited

droot left a comment

droot left a comment

mengqiy May 4, 2019 •

edited

mengqiy May 4, 2019

DirectXMan12 commented May 4, 2019

DirectXMan12 commented May 4, 2019

k8s-ci-robot commented May 4, 2019

k8s-ci-robot commented May 4, 2019

mengqiy commented May 4, 2019

⚠️ Marker Parsing and Structure #190

⚠️ Marker Parsing and Structure #190

Conversation

DirectXMan12 commented Apr 16, 2019 • edited

DirectXMan12 commented Apr 16, 2019 • edited

DirectXMan12 commented Apr 27, 2019

droot Apr 30, 2019

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

droot Apr 30, 2019

Choose a reason for hiding this comment

droot Apr 30, 2019

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

droot May 1, 2019

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

droot commented May 2, 2019

DirectXMan12 commented May 2, 2019

DirectXMan12 commented May 2, 2019 • edited

droot left a comment

Choose a reason for hiding this comment

droot May 3, 2019

Choose a reason for hiding this comment

mengqiy May 4, 2019 • edited

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

mengqiy May 4, 2019 • edited

Choose a reason for hiding this comment

mengqiy May 4, 2019

Choose a reason for hiding this comment

DirectXMan12 commented May 4, 2019

DirectXMan12 commented May 4, 2019

k8s-ci-robot commented May 4, 2019

k8s-ci-robot commented May 4, 2019

mengqiy commented May 4, 2019

DirectXMan12 commented Apr 16, 2019 •

edited

DirectXMan12 commented Apr 16, 2019 •

edited

DirectXMan12 commented May 2, 2019 •

edited

mengqiy May 4, 2019 •

edited

mengqiy May 4, 2019 •

edited