Upgrade Istio to 1.5.1

[mraszplewicz]

Hi,

I did some work to upgrade Istio version to 1.5.1. I know it is not perfect but maybe you can use it somehow. I use only model and needed AuthorizationPolicy so I added it. I have also upgraded network to v1beta1. Link to my commit: https://github.com/mraszplewicz/istio-java-api/commit/2fff582522077c59a83c1953d6ee47ba5f1f8c83

There were two issues while upgrading:

1. In CorsPolicy there are both allowOrigin and allowOrigins fields - I had to change istio-schema.json manually.
2. When I use StringMatch in allowOrigins or http.match It is trying to add matchType field in yaml. This is not a new problem - it was present also in current version.

Regards, Maciej Raszplewicz

[shalk]

Great job, recently i was trying to upgrade istio version to 1.5.1. First i changed go.mod and meet jq problem， fix generate.go， But when i make strict I meet a lot of warn...

1. ref istio api https://github.com/istio/api/blob/1.5.1/networking/v1beta1/virtual_service.proto#L1049 I think allowOrigin is deprecated but not reserved, so you should not remove it manually
2. I did not get what you mean

[metacosm]

Hi, thanks for this! I will look into it as soon as possible!

[mraszplewicz]

@shalk I also have some warnings while running make strict. I fixed whatever I could and then used make without strict.

1. The problem with allowOrigin and allowOrigins (both arrays) is that it just doesn't compile. Code generated from json schema has some errors.

   [ERROR] COMPILATION ERROR : 
   [INFO] -------------------------------------------------------------
   [ERROR] /home/maciek/src/istio-java-api/istio-model/target/generated-sources/annotations/me/snowdrop/istio/api/networking/v1beta1/CorsPolicyFluentImpl.java:[415,20] name clash: hasMatchingAllowOrigin(io.fabric8.kubernetes.api.builder.Predicate<me.snowdrop.istio.api.networking.v1beta1.StringMatchBuilder>) and hasMatchingAllowOrigin(io.fabric8.kubernetes.api.builder.Predicate<java.lang.String>) have the same erasure
   [ERROR] /home/maciek/src/istio-java-api/istio-model/target/generated-sources/annotations/me/snowdrop/istio/api/networking/v1beta1/CorsPolicyFluent.java:[126,20] name clash: hasMatchingAllowOrigin(io.fabric8.kubernetes.api.builder.Predicate<me.snowdrop.istio.api.networking.v1beta1.StringMatchBuilder>) and hasMatchingAllowOrigin(io.fabric8.kubernetes.api.builder.Predicate<java.lang.String>) have the same erasure

2. It renders wrong yaml while using string match field. It is something like:

     allowOrigins:
     - matchType:
         regex: example.com

   instead of:

     allowOrigins:
     - regex: example.com

   and

     uri:
       matchType:
         prefix: /testcontext

   instead of:

     uri:
       prefix: /testcontext

[FWiesner]

hi guys. I'de be highly interested in a fix to this update issue. My own Golang Kung-Fu is unfortunately not sufficient to get things working

[metacosm]

@mraszplewicz can you please open a PR?

[tanjunchen]

hello , everybody , Is there any progress about update istio to 1.5.1? @mraszplewicz for pr +1 . Thanks for your update.

[mraszplewicz]

I have opened a draft pull request. I do not consider this finished. It is just good enough for my use case.

[metacosm]

@mraszplewicz Thank you! Would you mind re-opening the PR agains the 1.5.x branch I just created? This way, I'll merge your changes and use them as starting point.

[mraszplewicz]

@metacosm I have changed the target to the 1.5.x branch

[cfryanr]

@metacosm and @mraszplewicz, We're excited to use the newest version and we saw that you merged the PR. 🎉

We tried checking out the 1.5.x branch and doing mvn clean install.

We noticed that there seems to be some missing generated code. There are no implementations of the interface me.snowdrop.istio.api.networking.v1alpha3.EnvoyConfigObjectMatch.ObjectTypes which corresponds to this oneof: https://github.com/istio/api/blob/e5412c253ffe940c588be743cc10e4904537c925/networking/v1alpha3/envoy_filter.proto#L556-L563

This prevents us from doing the following:

EnvoyFilter envoyFilter = new EnvoyFilterBuilder()
        .withNewMetadata()
        .withName("some-name")
        .withNamespace("some-namespace")
        .endMetadata()
        .withNewSpec()
        .withWorkloadSelector(new WorkloadSelectorBuilder().withLabels(labels).build())
        .withConfigPatches(
                new EnvoyConfigObjectPatchBuilder()
                        .withApplyTo(ApplyTo.HTTP_FILTER)
                        .withMatch(new EnvoyConfigObjectMatchBuilder()
                                .withObjectTypes(/* THERE IS NOTHING THAT WE CAN PASS HERE */)
                                .build())
                        .withPatch(new PatchBuilder()
                                .withOperation(Operation.INSERT_BEFORE)
                                .withValue(new StructBuilder().withFields(fields).build())
                                .build())
                        .build())
        .endSpec()
        .build();

We're new to the project so we're not sure how to fix the code generation ourselves.

[cfryanr]

Somewhat separately, and this might just be a question of not being familiar with how to use the library, how does one generate a nested Struct? We ask because we're new at using the library so we can't be sure if it is another missing aspect of the code generation, or if we just missed understanding something.

For example, in the code above, we're trying to make an EnvoyFilter which contains a patch that we would write in yaml like this:

patch:
  operation: INSERT_BEFORE
  value:
    name: envoy.ext_authz
    config:
      stat_prefix: ext_authz
      grpc_service:
        envoy_grpc:
          cluster_name: ext_authz
        timeout: 10s

In the EnvoyFilter proto file, the value of the patch is a field of type google.protobuf.Struct. Normally a Struct is a key/value map where the value at any key is allowed to be another Struct, so you can create nested values.

But me.snowdrop.istio.api.Struct only has a map of String to me.snowdrop.istio.api.cexl.TypedValue, and each TypedValue is made up of a type and a string, so it's not clear how to represent a nested map like the one shown in the example above.

Here's a best guess. Is this even close?

Map<String, TypedValue> fields = new HashMap<>();
fields.put("name", new TypedValueBuilder()
        .withType(ValueType.STRING)
        .withExpression("envoy.ext_authz")
        .build());
fields.put("config", new TypedValueBuilder()
        .withType(ValueType.STRING_MAP)
        .withExpression(("{" +
                "  `stat_prefix`: `ext_authz`," +
                "  `grpc_service`: {" +
                "    `envoy_grpc`: {" +
                "      `cluster_name`: `ext_authz`" +
                "    }," +
                "    `timeout`: `10s`" +
                "  }" +
                "}").replace('`', '"'))
        .build());

Appreciate any guidance!

[metacosm]

@cfryanr I’m still working on the 1.5.x branch. I had some ideas on how to improve the schema generation and started working on this but I think I will set them aside for now in favor of a quick 1.5 beta release…

[metacosm]

I've pushed several changes to the branch, in particular to be able to support multiple versions of the same CR (e.g. to support both v1alpha3 and v1beta1 versions of networking). I've also handled the allowOrigin vs. allowOrigins case by renaming the allowOrigin field, which is deprecated, to deprecatedAllowOrigin to avoid confusing both the code generator and the api… 😄 I will take a look at the issues that @cfryanr reported tomorrow and hopefully will release a beta version with 1.5.4 support as well.

[metacosm]

@cfryanr the issue with ObjectTypes should be fixed now. I'm going to look at the Struct issue.

[metacosm]

@cfryanr the issue with nested Structs in EnvoyFilter should also be fixed now

[metacosm]

I've released 1.5.4-Alpha1. Please give it a try and open new issues as needed.

[cfryanr]

Thanks for the fixes, @metacosm! We'll give them a try as soon as we can.

[metacosm]

@cfryanr any news?

[cfryanr]

Hi @metacosm, looks like we're not going to get back to that right away. Thanks for the fixes, though. We'll be sure to try them out when we get back to that project.

[shalk]

I just update my project dependency from 1.1.1 to 1.5.4-Alpha1,and switch api from v1alpha3 to v1beta1. It works fine

<dependency>
    <groupId>me.snowdrop</groupId>
    <artifactId>istio-client</artifactId>
    <version>1.5.4-Alpha1</version>
</dependency>

[metacosm]

@shalk thank you for the update! I'm plannning on releasing 1.5.4 final this week and get on 1.6.1 support…

[mraszplewicz]

Hi @metacosm,

I have also upgraded to 1.5.4-Alpha1 and almost everything works. I have found two bugs but I don't have enough time to write tests. Both are mentioned in my first post on this issue. I will create new issues for them and write tests as soon as I can.

[metacosm]

@mraszplewicz is it regarding allowOrigin vs. allowOrigins? If yes, I've addressed this by renaming allowOrigin to deprecatedAllowOrigin. See https://github.com/snowdrop/istio-java-api/issues/83#issuecomment-630450762

[mraszplewicz]

@metacosm the first one is regarding allowOrigins. Now it serializes to deprecatedAllowOrigin... so I had to manually set deprecatedAllowOrigin to null.

The second one is a problem with StringMatch serialization.

Anyway, I use it and I think it is good enough to release it.

[metacosm]

I see. I didn’t consider that aspect… 😔 Let’s see if I can fix it. Do you have more details on the StringMatch issue? I thought I had addressed one such issue but maybe I missed some.

[mraszplewicz]

Look at yaml fragments in https://github.com/snowdrop/istio-java-api/issues/83#issuecomment-618953816 as far as I remember Istio will not accept matchType in yaml.