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

chore: v1 api #544

Merged
merged 12 commits into from
Jan 6, 2023
Merged

chore: v1 api #544

merged 12 commits into from
Jan 6, 2023

Conversation

pmengelbert
Copy link
Contributor

@pmengelbert pmengelbert commented Dec 15, 2022

Create v1 api

In order to create the v1 api for eraser, a number of additional changes had to be made.

Our eraser-tooling image had to be modified to include the conversion-gen tool. This is to generate type conversions between the older v1alpha1 API and the newer v1.

To facilitate conversion between the two, an unversioned api was created. This becomes a common hub between v1 and v1alpha1 and facilitates conversion. It also makes it easier for our code: with the unversioned api, we no longer need to update large swaths of code when a new api version is introduced.

In order to get the conversion-gen tool to work, I needed to add several magic comments throughout the api code. Most importantly, I needed to add a doc.go file to each of the api version directories. conversion-gen looks for this file and will not work without it. Above the package name in those files, I added the following two magic comments:

+// +k8s:conversion-gen=github.com/Azure/eraser/api/unversioned
+// -external-types=github.com/Azure/eraser/api/v1

This tells conversion-gen to use the unversioned api as the conversion hub.

To skip kubebuilder processing the unversioned api, +kubebuilder:skip had to be added before the package declaration. To specify which api version should be stored in etcd, the +kubebuilder:storageversion comment was added to the v1 types. In addition, kubebuilder:deprecatedversion:warning="v1alpha1 of the eraser API has been deprecated" was added to the v1alpha1 API in order to warn the user to upgrade.

In the groupversion_info.go file of each version directory, I had to add the line

localSchemeBuilder = runtime.NewSchemeBuilder(SchemeBuilder.AddToScheme)

To be perfectly honest, I'm not sure what this does, but it is required for the conversion-gen process to generate working code.

Finally, a new file was added to test/e2e/test-data which deploys an imagelist using the new API version. One of the tests was updated to use this file instead.

Signed-off-by: Peter Engelbert pmengelbert@gmail.com

Makefile Outdated Show resolved Hide resolved
api/unversioned/imagejob_types.go Outdated Show resolved Hide resolved
test/e2e/test-data/eraser_v1_imagelist.yaml Show resolved Hide resolved
build/tooling/Dockerfile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
api/v1/groupversion_info.go Outdated Show resolved Hide resolved
api/v1/groupversion_info.go Show resolved Hide resolved
api/v1/imagelist_types.go Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2022

Codecov Report

Merging #544 (bbfd528) into main (45f5eb2) will not change coverage.
The diff coverage is 21.42%.

@@           Coverage Diff           @@
##             main     #544   +/-   ##
=======================================
  Coverage   15.22%   15.22%           
=======================================
  Files          13       13           
  Lines        1511     1511           
=======================================
  Hits          230      230           
  Misses       1255     1255           
  Partials       26       26           
Flag Coverage Δ
unittests 15.22% <21.42%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/eraser/eraser.go 0.00% <0.00%> (ø)
pkg/scanners/trivy/trivy.go 0.00% <0.00%> (ø)
pkg/scanners/trivy/types.go 0.00% <0.00%> (ø)
pkg/utils/utils.go 11.83% <0.00%> (ø)
pkg/eraser/helpers.go 72.22% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pmengelbert pmengelbert force-pushed the chore/v1_api branch 2 times, most recently from f97fd49 to 252145c Compare December 16, 2022 16:34
@pmengelbert pmengelbert marked this pull request as ready for review December 16, 2022 16:35
@pmengelbert pmengelbert force-pushed the chore/v1_api branch 3 times, most recently from 481ac6d to f5e49c8 Compare December 16, 2022 22:40
@pmengelbert pmengelbert marked this pull request as draft December 29, 2022 22:09
@pmengelbert pmengelbert force-pushed the chore/v1_api branch 7 times, most recently from 9ca9e92 to dfd8a0e Compare January 5, 2023 17:52
@pmengelbert pmengelbert marked this pull request as ready for review January 5, 2023 17:52
@pmengelbert
Copy link
Contributor Author

Rebased on main and ready for review!

In order to create the v1 api for eraser, a number of additional changes
had to be made.

Our eraser-tooling image had to be modified to include the
`conversion-gen` tool. This is to generate type conversions between the
older `v1alpha1` API and the newer `v1`.

To facilitate conversion between the two, an `unversioned` api was
created. This becomes a common hub between `v1` and `v1alpha1` and
facilitates conversion. It also makes it easier for our code: with the
`unversioned` api, we no longer need to update large swaths of code when
a new api version is introduced.

In order to get the `conversion-gen` tool to work, I needed to add
several magic comments throughout the api code. Most importantly, I
needed to add a `doc.go` file to each of the api version directories.
`conversion-gen` looks for this file and will not work without it. Above
the package name in those files, I added the following two magic
comments:

```
+// +k8s:conversion-gen=github.com/Azure/eraser/api/unversioned
+// -external-types=github.com/Azure/eraser/api/v1
```

This tells `conversion-gen` to use the `unversioned` api as the
conversion hub.

To skip kubebuilder processing the `unversioned` api,
`+kubebuilder:skip` had to be added before the package declaration. To
specify which api version should be stored in etcd, the
`+kubebuilder:storageversion` comment was added to the `v1` types. In
addition, `kubebuilder:deprecatedversion:warning="v1alpha1 of the eraser API has been deprecated"`
was added to the `v1alpha1` API in order to warn the user to upgrade.

In the `groupversion_info.go` file of each version directory, I had to
add the line

```go
localSchemeBuilder = runtime.NewSchemeBuilder(SchemeBuilder.AddToScheme)
```

To be perfectly honest, I'm not sure what this does, but it is required
for the `conversion-gen` process to generate working code.

Finally, a new file was added to `test/e2e/test-data` which deploys an
imagelist using the new API version. One of the tests was updated to use
this file instead.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
In order to avoid changing the Makefile every time  a new version is
added, add an empty package to the `./api` directory in order to use
the `...` wildcard.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
This flag will no longer be needed, because we have reduced the size of
the CRDs.

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Makefile Outdated Show resolved Hide resolved
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM

@sozercan
Copy link
Member

sozercan commented Jan 6, 2023

looks like we might need a final make manifests

Signed-off-by: Peter Engelbert <pmengelbert@gmail.com>
@pmengelbert pmengelbert merged commit 9f75b7d into eraser-dev:main Jan 6, 2023
@pmengelbert pmengelbert mentioned this pull request Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants