Support of Istio Client API #3613

Sgitario · 2021-11-24T14:13:07Z

This PR supports v1alpha3 and v1beta1 Istio API
Solution is based on https://github.com/snowdrop/istio-java-api
Tests have been moved from the above repository to here.
Fixes #3593

Apart from adding the Istio API, we added two changes to the GO generator:

Ability to extract the field name from the protobug tag: this is optional and only enabled by a flag
Fix mapping of float numbers
These changes are in separated commits.

Sgitario · 2021-11-24T14:16:06Z

fyi @metacosm

rohanKanojia · 2021-11-24T14:49:34Z

Have you copy-pasted code from Istio Java API? Please preserve commit history We did something similar while porting Service Catalog extension #1605 (review)

Sgitario · 2021-11-24T15:43:10Z

Have you copy-pasted code from Istio Java API? Please preserve commit history We did something similar while porting Service Catalog extension #1605 (review)

I didn't copy pasted code from the snowdrop repo. I only used the snowdrop repo as a reference ++ migrated the tests to here.

extensions/istio/generator-v1alpha3/go.mod

.../istio/model-v1alpha3/src/main/java/io/fabric8/istio/api/networking/v1alpha3/CorsPolicy.java

manusa

LGTM, with some comments:

I think we can fix the most of the test related code smells
Some tests are failing on Windows

Sgitario · 2021-11-26T10:08:39Z

FYI, I'm investigating the issue with windows.

Sgitario · 2021-11-26T13:35:33Z

Fixed the Windows check

rbaeurle · 2021-12-03T09:29:11Z

I could verify that this PR seems to work fine for us.

Sometimes the go struct is defined like: ```go type MyClass struct { GoField *GoType `protobuf:"...,json=goField,proto3" json:"go_field,omitempty"` } ``` The json field is wrong ("go_field") and the protobuf field is right "goField". I would say that this is a wrong model, however this is a solution to pick the values from the protobuf section.

The "float" is mapped to Object by the json2pojo plugin. Using "number" will map the property to "double".

This PR supports v1alpha3 and v1beta1 Istio API Solution is based on https://github.com/snowdrop/istio-java-api Tests have been moved from the above repository to here. Fixes fabric8io#3593

sonarqubecloud · 2021-12-10T14:54:13Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
86 Code Smells

0.0% Coverage
12.0% Duplication

metacosm

Great job overall!
A few comments, though:

We probably need to document how to migrate to this new version since some method names have changed and, obviously, the packages are now different
This PR only covers the networking part of the Istio API. While this should cover quite a bit of what people use, there are several other APIs that would need to be ported/updated to match what was available in the istio-java-api project (mesh, policy, rbac, security and mixer APIs are missing). Not all of them might need to be ported since some of these are actually obsolete but this is still something that needs to be investigated.
The istio-java-api project also provided a CEL (https://opensource.google/projects/cel) parser via ANTLR to support validation of some of the configuration fields. Relevance to this new version needs to be assessed as well.
Basically, we need someone who routinely uses Istio to support this API to make sure that the relevant parts are covered correctly.

README.md

extensions/istio/client/pom.xml

extensions/istio/client/src/main/java/io/fabric8/istio/client/DefaultIstioClient.java

extensions/istio/client/src/main/java/io/fabric8/istio/client/V1alpha3APIGroupDSL.java

rohanKanojia · 2021-12-14T09:40:22Z

...ons/istio/client/src/main/java/io/fabric8/istio/client/V1alpha3APIGroupExtensionAdapter.java

+
+  @Override
+  public Class<V1alpha3APIGroupDSL> getExtensionType() {
+    return V1alpha3APIGroupDSL.class;


Suggested change

return V1alpha3APIGroupDSL.class;

return V1beta1APIGroupClient.class;

...ions/istio/client/src/main/java/io/fabric8/istio/client/V1beta1APIGroupExtensionAdapter.java

extensions/istio/tests/src/test/java/io/fabric8/istio/test/v1alpha3/DestinationRuleTest.java

rohanKanojia · 2021-12-14T10:19:35Z

There doesn't seem to be any CRUD test for any resource. Would be nice to add some Crud tests to test suite as well.

Sgitario · 2021-12-14T15:46:10Z

There doesn't seem to be any CRUD test for any resource. Would be nice to add some Crud tests to test suite as well.

Thanks for your review! I will update the PR asap.
One question about the missing CRUD tests: we have tests for create/delete/get only for the latest api v1beta1, example: https://github.com/fabric8io/kubernetes-client/blob/7f9cb03a01d37015268a35ccffe824f7c82f009d/extensions/istio/tests/src/test/java/io/fabric8/istio/test/v1beta1/AuthorizationPolicyTest.java

Do you mean to also cover updates in these resources or something else?

rohanKanojia · 2021-12-14T15:57:42Z

Sorry for not being clear, I was referring to CRUD mode in MockServer. You don't need to add expectations in this mode. Here is an example:
https://github.com/fabric8io/kubernetes-client/blob/master/extensions/camel-k/tests/src/test/java/io/fabric8/camelk/test/crud/IntegrationCrudTest.java#L29

manusa · 2021-12-14T16:21:26Z

Sorry, I didn't read the comments, before merging (just saw the approval).

Could you add the requested changes to a new PR?

Sgitario · 2021-12-15T05:18:08Z

Sorry for not being clear, I was referring to CRUD mode in MockServer. You don't need to add expectations in this mode. Here is an example: https://github.com/fabric8io/kubernetes-client/blob/master/extensions/camel-k/tests/src/test/java/io/fabric8/camelk/test/crud/IntegrationCrudTest.java#L29

ok ok! I've added the crud flag to true in the v1beta1 VirtualService test.

This PR fixes all the requested changes by Rohan in fabric8io#3613.

Sgitario · 2021-12-15T05:40:51Z

Request changes fixed in #3650

This PR fixes all the requested changes by Rohan in #3613.

rohanKanojia requested review from metacosm and iocanel November 24, 2021 14:30

rohanKanojia linked an issue Nov 24, 2021 that may be closed by this pull request

Move Snowdrop Istio extension to Fabric8 Kubernetes Client Extensions #3493

Closed

metacosm reviewed Nov 24, 2021

View reviewed changes

extensions/istio/generator-v1alpha3/go.mod Outdated Show resolved Hide resolved

Sgitario requested a review from metacosm November 25, 2021 06:08

manusa reviewed Nov 26, 2021

View reviewed changes

.../istio/model-v1alpha3/src/main/java/io/fabric8/istio/api/networking/v1alpha3/CorsPolicy.java Show resolved Hide resolved

manusa approved these changes Nov 26, 2021

View reviewed changes

Sgitario force-pushed the good_istio branch from 9b1d189 to 77d9d74 Compare November 26, 2021 10:14

manusa requested a review from rohanKanojia November 29, 2021 12:20

metacosm mentioned this pull request Dec 1, 2021

chore: initial update to sundrio 0.60.0, Fabric8 5.10.1, istio 1.7.8 snowdrop/istio-java-api#141

Draft

3 tasks

Sgitario added 7 commits December 7, 2021 08:27

Fix "float" mapping in go generator

7611360

The "float" is mapped to Object by the json2pojo plugin. Using "number" will map the property to "double".

Support Istio in Kubernetes Client

e9eeb6e

This PR supports v1alpha3 and v1beta1 Istio API Solution is based on https://github.com/snowdrop/istio-java-api Tests have been moved from the above repository to here. Fixes fabric8io#3593

Istio: Update documentation

ad8d174

Istio: Use released version of istio-client and latest of istio-api

27f226b

Istio: Polish code smells

6753c4e

Istio: Fix conflicting name for LoadBalancer HttpCookie spotted by Win

7ddbeca

Sgitario force-pushed the good_istio branch from 35a577d to 7ddbeca Compare December 7, 2021 07:29

refactor: use nested builders

7f9cb03

metacosm approved these changes Dec 10, 2021

View reviewed changes

manusa added this to the 5.11.0 milestone Dec 14, 2021

rohanKanojia approved these changes Dec 14, 2021

View reviewed changes

manusa merged commit 1478524 into fabric8io:master Dec 14, 2021

Sgitario deleted the good_istio branch December 15, 2021 04:59

Sgitario added a commit to Sgitario/kubernetes-client that referenced this pull request Dec 15, 2021

Istio Client: Amend requested changes by Rohan

19e0fe7

This PR fixes all the requested changes by Rohan in fabric8io#3613.

Sgitario mentioned this pull request Dec 15, 2021

Istio Client: Amend requested changes by Rohan #3650

Merged

manusa pushed a commit that referenced this pull request Dec 15, 2021

Istio Client: Amend requested changes by Rohan

3627dff

This PR fixes all the requested changes by Rohan in #3613.

rbaeurle mentioned this pull request Jan 3, 2022

IllegalStateException while reading VirtualServices #3709

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support of Istio Client API #3613

Support of Istio Client API #3613

Sgitario commented Nov 24, 2021

Sgitario commented Nov 24, 2021

rohanKanojia commented Nov 24, 2021 •

edited

Loading

Sgitario commented Nov 24, 2021 •

edited

Loading

manusa left a comment

Sgitario commented Nov 26, 2021

Sgitario commented Nov 26, 2021

rbaeurle commented Dec 3, 2021

sonarqubecloud bot commented Dec 10, 2021

metacosm left a comment

rohanKanojia Dec 14, 2021

rohanKanojia commented Dec 14, 2021

Sgitario commented Dec 14, 2021

rohanKanojia commented Dec 14, 2021

manusa commented Dec 14, 2021

Sgitario commented Dec 15, 2021

Sgitario commented Dec 15, 2021

	return V1alpha3APIGroupDSL.class;
	return V1beta1APIGroupClient.class;

Support of Istio Client API #3613

Support of Istio Client API #3613

Conversation

Sgitario commented Nov 24, 2021

Sgitario commented Nov 24, 2021

rohanKanojia commented Nov 24, 2021 • edited Loading

Sgitario commented Nov 24, 2021 • edited Loading

manusa left a comment

Choose a reason for hiding this comment

Sgitario commented Nov 26, 2021

Sgitario commented Nov 26, 2021

rbaeurle commented Dec 3, 2021

sonarqubecloud bot commented Dec 10, 2021

metacosm left a comment

Choose a reason for hiding this comment

rohanKanojia Dec 14, 2021

Choose a reason for hiding this comment

rohanKanojia commented Dec 14, 2021

Sgitario commented Dec 14, 2021

rohanKanojia commented Dec 14, 2021

manusa commented Dec 14, 2021

Sgitario commented Dec 15, 2021

Sgitario commented Dec 15, 2021

rohanKanojia commented Nov 24, 2021 •

edited

Loading

Sgitario commented Nov 24, 2021 •

edited

Loading