Add init code for the istio provider #99

dpasiukevich · 2023-11-26T23:30:30Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adds initial code for istio provider. This PR adds the interface implementation and helpers to read istio CRDs from the cluster. The actual conversion would be done in the following PRs.

In detailed, this PR:

Reads istio Gateway & VirtualServices
Removes customResources arg from the Provider interface. Currently is passed to the provider as a interface{} (nil value & nil type). Provider code currently cannot modify this struct (because cannot cast to any other type and cannot overwrite the value).
The PR simplifies interface and removes the need to pass customResources as interface{} which is also shared across different providers.
Now the provider if wants to handle custom resources, must itself parse them and store in any desired format locally until ToGatewayAPI is called on it.
Updates go.mod to import latest IstioAPI.
Bump linter version and fixes newly lint findings in the repo.

cc @howardjohn

Does this PR introduce a user-facing change?:

[Istio provider] set up code for reading istio custom resources

dpasiukevich · 2023-11-26T23:36:23Z

/test pull-ingress2gateway-verify

dpasiukevich · 2023-11-27T00:26:16Z

/assign @LiorLieberman

LiorLieberman · 2023-11-27T09:56:36Z

Thats great @dpasiukevich !
I will book some time today to review this.

In the meantime, can you edit the release-notes section to reflect the changes (that will then be included in our release notes)

This is a user facing change assuming now users will be able to invoke the tool with istio provider.

/cc @levikobi @mlavacca

dpasiukevich · 2023-11-27T10:05:36Z

The current PR is just a basic foundation to put the actual code later on.
I planned to add the release notes to notify customers when the converter would be ready at least on the basic level.

LiorLieberman

Overall looks good to me and thanks for improving the interface! I left a few small comments.

This PR actually looks like it is half implementing conversion (I can see you read storage.Gateways and fill in some metadata while the spec and virtual services are not being read). Ideally it should either set up the init code and have a followup PR for the conversion itslef OR doing everything in this PR.

pkg/i2gw/providers/istio/converter.go

pkg/i2gw/providers/istio/resource_reader.go

pkg/i2gw/providers/istio/istio.go

LiorLieberman · 2023-11-27T14:12:49Z

go.mod

+	google.golang.org/genproto v0.0.0-20231120223509-83a465c0220f // indirect
+	google.golang.org/genproto/googleapis/api v0.0.0-20231120223509-83a465c0220f // indirect
+	google.golang.org/protobuf v1.31.0 // indirect
+	gopkg.in/evanphx/json-patch.v5 v5.7.0 // indirect


I wonder where this comes from as we already have above

github.com/evanphx/json-patch v5.7.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.7.0 // indirect

It may be from istio.io/api v1.20.0. It's needed for the new provider.

Its from sigs.k8s.io/kustomize/api which was updated.

Istio API has no new deps:

github.com/golang/protobuf v1.5.3 google.golang.org/genproto/googleapis/api v0.0.0-20230920204549-e6e6cdab5c13 google.golang.org/genproto/googleapis/rpc v0.0.0-20231009173412-8bfb1ae86b6c google.golang.org/grpc v1.58.3 google.golang.org/protobuf v1.31.0 k8s.io/api v0.28.0 k8s.io/apimachinery v0.28.0

just upgraded to 0.28

pkg/i2gw/providers/istio/converter.go

dpasiukevich

@LiorLieberman updated in regards for the PR comments.

As for this PR -- I just want to have a proper basis and agreement for the conversion logic (imports, proper interface for the Provider).

If everything was done in one PR (this would've been a gargantuan PR). As I cannot reuse any existing conversion logic in the repo (it's for Ingress, while the provider is for Istio).

I'd prefer quicker&smaller iterations.

If needed I can remove the provider from registration for now.

pkg/i2gw/providers/istio/converter.go

pkg/i2gw/providers/istio/istio.go

pkg/i2gw/providers/istio/resource_reader.go

dpasiukevich · 2023-11-27T16:33:16Z

pkg/i2gw/providers/istio/types.go

+	VirtualServiceKind = "VirtualService"
+)
+
+type gateway struct {


@howardjohn would this be an ok representation of the object to parse into from the unstructured.Unstructured? Or is there something similar in the istio API (adapted Istio resources for K8S)?

I didn't find any, asking just to double check.

istio/client-go repo already has these that can be used

Cool, exactly what's needed. Thanks!

pkg/i2gw/providers/ingressnginx/resource_reader.go

mlavacca

Thanks for it @dpasiukevich, lgtm!

Will defer to @levikobi for the final lgtm stamp.

dpasiukevich · 2023-11-30T20:26:14Z

/test pull-ingress2gateway-test

pkg/i2gw/providers/istio/istio.go

dpasiukevich · 2023-12-01T10:59:01Z

/label tide/merge-method-squash

pkg/i2gw/providers/ingressnginx/resource_fetcher.go

pkg/i2gw/providers/istio/resource_fetcher.go

LiorLieberman · 2023-12-01T10:58:44Z

pkg/i2gw/providers/istio/types.go

+package istio
+
+const (
+	APIVersion         = "networking.istio.io/v1beta1"


maybe APIVersionV1Beta1 as istio also have alpha3 IIRC

I'd keep as is to indicate the only currently supported API version is networking.istio.io/v1beta1 for istio converter

pkg/i2gw/providers/kong/resource_fetcher.go

LiorLieberman · 2023-12-01T11:06:35Z

/lgtm
/approve

k8s-ci-robot · 2023-12-01T11:06:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dpasiukevich, LiorLieberman, mlavacca

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

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

* Add init code for the istio provider * rename resource_readers * use istio api types * return readers back * refactor upd * rename filenames to readers

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 26, 2023

k8s-ci-robot requested review from levikobi and Xunzhuo November 26, 2023 23:30

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 26, 2023

dpasiukevich had a problem deploying to release November 26, 2023 23:30 — with GitHub Actions Failure

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 26, 2023

dpasiukevich force-pushed the framework_istio_provider branch from 8fa45df to 4e5c708 Compare November 26, 2023 23:33

dpasiukevich temporarily deployed to release November 26, 2023 23:33 — with GitHub Actions Inactive

dpasiukevich force-pushed the framework_istio_provider branch from 4e5c708 to 4ecaca9 Compare November 26, 2023 23:42

dpasiukevich temporarily deployed to release November 26, 2023 23:43 — with GitHub Actions Inactive

dpasiukevich force-pushed the framework_istio_provider branch from 4ecaca9 to bbe18f5 Compare November 26, 2023 23:54

dpasiukevich temporarily deployed to release November 26, 2023 23:54 — with GitHub Actions Inactive

dpasiukevich force-pushed the framework_istio_provider branch from bbe18f5 to 5afb0e2 Compare November 27, 2023 00:10

dpasiukevich temporarily deployed to release November 27, 2023 00:10 — with GitHub Actions Inactive

k8s-ci-robot assigned LiorLieberman Nov 27, 2023

k8s-ci-robot requested a review from mlavacca November 27, 2023 09:56

LiorLieberman linked an issue Nov 27, 2023 that may be closed by this pull request

Add Istio provider support #100

Closed

LiorLieberman reviewed Nov 27, 2023

View reviewed changes

dpasiukevich force-pushed the framework_istio_provider branch from 5afb0e2 to df62149 Compare November 27, 2023 14:54

dpasiukevich temporarily deployed to release November 27, 2023 14:54 — with GitHub Actions Inactive

dpasiukevich commented Nov 27, 2023

View reviewed changes

pkg/i2gw/providers/istio/converter.go Outdated Show resolved Hide resolved

pkg/i2gw/providers/istio/istio.go Outdated Show resolved Hide resolved

pkg/i2gw/providers/istio/resource_reader.go Outdated Show resolved Hide resolved

dpasiukevich force-pushed the framework_istio_provider branch from df62149 to 71727d1 Compare November 27, 2023 16:29

dpasiukevich temporarily deployed to release November 27, 2023 16:29 — with GitHub Actions Inactive

dpasiukevich commented Nov 27, 2023

View reviewed changes

dpasiukevich force-pushed the framework_istio_provider branch from 71727d1 to 165f869 Compare November 27, 2023 16:50

dpasiukevich temporarily deployed to release November 27, 2023 16:50 — with GitHub Actions Inactive

dpasiukevich force-pushed the framework_istio_provider branch from 4bc0d87 to 78baf25 Compare November 30, 2023 14:59

dpasiukevich temporarily deployed to release November 30, 2023 15:00 — with GitHub Actions Inactive

Add init code for the istio provider

29b4c50

dpasiukevich force-pushed the framework_istio_provider branch from 78baf25 to 29b4c50 Compare November 30, 2023 15:10

dpasiukevich temporarily deployed to release November 30, 2023 15:11 — with GitHub Actions Inactive

mlavacca reviewed Nov 30, 2023

View reviewed changes

pkg/i2gw/providers/ingressnginx/resource_reader.go Outdated Show resolved Hide resolved

pkg/i2gw/providers/ingressnginx/resource_reader.go Outdated Show resolved Hide resolved

pkg/i2gw/providers/ingressnginx/resource_reader.go Outdated Show resolved Hide resolved

rename resource_readers

587b7d2

dpasiukevich temporarily deployed to release November 30, 2023 15:36 — with GitHub Actions Inactive

mlavacca approved these changes Nov 30, 2023

View reviewed changes

use istio api types

483422d

dpasiukevich temporarily deployed to release November 30, 2023 20:11 — with GitHub Actions Inactive

LiorLieberman reviewed Dec 1, 2023

View reviewed changes

pkg/i2gw/providers/istio/istio.go Outdated Show resolved Hide resolved

LiorLieberman reviewed Dec 1, 2023

View reviewed changes

pkg/i2gw/providers/istio/istio.go Outdated Show resolved Hide resolved

return readers back

7dd1150

dpasiukevich temporarily deployed to release December 1, 2023 10:45 — with GitHub Actions Inactive

refactor upd

ad84be4

dpasiukevich temporarily deployed to release December 1, 2023 10:48 — with GitHub Actions Inactive

mlavacca approved these changes Dec 1, 2023

View reviewed changes

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 1, 2023

LiorLieberman reviewed Dec 1, 2023

View reviewed changes

rename filenames to readers

21ade9c

dpasiukevich temporarily deployed to release December 1, 2023 11:01 — with GitHub Actions Inactive

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 1, 2023

k8s-ci-robot merged commit 6ff1a24 into kubernetes-sigs:main Dec 1, 2023
1 check passed

LiorLieberman mentioned this pull request Dec 26, 2023

Duplicate resources when using implementation-specific annotations #109

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add init code for the istio provider #99

Add init code for the istio provider #99

dpasiukevich commented Nov 26, 2023 •

edited by LiorLieberman

Loading

dpasiukevich commented Nov 26, 2023

dpasiukevich commented Nov 27, 2023

LiorLieberman commented Nov 27, 2023

dpasiukevich commented Nov 27, 2023

LiorLieberman left a comment •

edited

Loading

LiorLieberman Nov 27, 2023

dpasiukevich Nov 27, 2023

howardjohn Nov 30, 2023

dpasiukevich left a comment

dpasiukevich Nov 27, 2023

howardjohn Nov 30, 2023

dpasiukevich Nov 30, 2023

mlavacca left a comment

dpasiukevich commented Nov 30, 2023

dpasiukevich commented Dec 1, 2023

LiorLieberman Dec 1, 2023

dpasiukevich Dec 1, 2023

LiorLieberman commented Dec 1, 2023

k8s-ci-robot commented Dec 1, 2023

Add init code for the istio provider #99

Add init code for the istio provider #99

Conversation

dpasiukevich commented Nov 26, 2023 • edited by LiorLieberman Loading

dpasiukevich commented Nov 26, 2023

dpasiukevich commented Nov 27, 2023

LiorLieberman commented Nov 27, 2023

dpasiukevich commented Nov 27, 2023

LiorLieberman left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpasiukevich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlavacca left a comment

Choose a reason for hiding this comment

dpasiukevich commented Nov 30, 2023

dpasiukevich commented Dec 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LiorLieberman commented Dec 1, 2023

k8s-ci-robot commented Dec 1, 2023

dpasiukevich commented Nov 26, 2023 •

edited by LiorLieberman

Loading

LiorLieberman left a comment •

edited

Loading