(feat-ca) Ca rotation time to support months and year #750

tharun208 · 2020-05-19T12:18:43Z

Summary

Currently, Certificate Rotation times use proto duration which has limitation to hours, This pr extends this and we can able to create rotation time for a month, year, etc.

Full changelog

Updated mesh proto definitions
Updated expiration time implementations

Issues resolved

Fix #743

nickolaev

Looks good to me, with a small testing suggestion.

@lobkovilya is there a proper place to document this new format?

pkg/sds/server/server_test.go

jakubdyszkiewicz

Please add validation that value is valid in mesh_validator.go

jakubdyszkiewicz

The task was to actually change this in BuiltinCertificateAuthorityConfig, but we need this in both places, so that's a good direction!

Also: is there maybe a way to plug in custom google.protobuf.Duration unmarshaller that will work with years? This will be more clean way of solving this. Otherwise we need to do the parsing ourselves everytime. We could also go with our custom Duration that supports years

tharun208 · 2020-05-20T09:08:33Z

The task was to actually change this in BuiltinCertificateAuthorityConfig, but we need this in both places, so that's a good direction!

Also: is there maybe a way to plug in custom google.protobuf.Duration unmarshaller that will work with years? This will be more clean way of solving this. Otherwise we need to do the parsing ourselves everytime. We could also go with our custom Duration that supports years

I am just wondering if we use our custom Duration, we need to do 2 levels of parsing time information for the above.

jakubdyszkiewicz · 2020-05-20T10:49:13Z

I am just wondering if we use our custom Duration, we need to do 2 levels of parsing time information for the above.

can you elaborate on this? Thanks

lobkovilya · 2020-05-20T13:03:27Z

@nickolaev as far as I know we document all format related things in *.proto files

Updated ca plugins to parseDuration Updated tests Fix kumahq#743

Updated builtinca manager to parseDuration Updated tests Fix kumahq#743

Fix kumahq#743

tharun208 · 2020-05-28T10:22:23Z

can I add changelog for this as a seperate PR ?

nickolaev · 2020-05-28T11:16:24Z

We're going to generate the changelog semi-automated at release time.

subnetmarco · 2020-05-30T05:43:54Z

It seems like it's not working when trying with the current version of kumactl.

$ echo "type: Mesh
> name: default
> mtls:
>   enabledBackend: ca-1
>   backends:
>     - name: ca-1
>       type: builtin
>       dpCert:
>         rotation:
>           expiration: 1d
>       conf:
>         caCert:
>           RSAbits: 2048
>           expiration: 10y" | kumactl apply -f -
Error: YAML contains invalid resource: bad Duration: time: unknown unit d in duration 1d

When trying with a kumactl freshly compiled from master:

$ echo "type: Mesh
name: default
mtls:
  enabledBackend: ca-1
  backends:
    - name: ca-1
      type: builtin
      dpCert:
        rotation:
          expiration: 1d
      conf:
        caCert:
          RSAbits: 2048
          expiration: 10y" | ./kumactl apply -f -
Error: Could not update a resource (Resource is not valid)
* mtls.dpcert.rotation.expiration: has to be a valid format

and if I replace 1d with 24h but I keep the 10y below, kuma-cp logs the following error:

2020-05-29T22:44:55.304-0700 ERROR Could not update a resource {"error": "could not create CA of backend name ca-1: failed to create CA for mesh "default" and backend "ca-1": time: unknown unit y in duration 10y", "errorVerbose": "time: unknown unit y in duration 10y\nfailed to create CA for mesh "default" and backend "ca-1"\ngithub.com/Kong/kuma/pkg/plugins/ca/builtin.(*builtinCaManager).Ensure\n\t/Users/marco/git/kuma/pkg/plugins/ca/builtin/manager.go:40\ngithub.com/Kong/kuma/pkg/core/managers/apis/mesh.EnsureEnabledCA\n\t/Users/marco/git/kuma/pkg/core/managers/apis/mesh/mesh_helpers.go:45\ngithub.com/Kong/kuma/pkg/core/managers/apis/mesh.(*meshManager).Update\n\t/Users/marco/git/kuma/pkg/core/managers/apis/mesh/mesh_manager.go:136\ngithub.com/Kong/kuma/pkg/core/resources/manager.(*customizableResourceManager).Update\n\t/Users/marco/git/kuma/pkg/core/resources/manager/customizable_manager.go:43\ngithub.com/Kong/kuma/pkg/api-server.(*resourceEndpoints).updateResource\n\t/Users/marco/git/kuma/pkg/api-server/resource_endpoints.go:144\ngithub.com/Kong/kuma/pkg/api-server.(*resourceEndpoints).createOrUpdateResource\n\t/Users/marco/git/kuma/pkg/api-server/resource_endpoints.go:128\ngithub.com/emicklei/go-restful.(*FilterChain).ProcessFilter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/filter.go:21\ngithub.com/emicklei/go-restful.CrossOriginResourceSharing.Filter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/cors_filter.go:40\ngithub.com/emicklei/go-restful.(*FilterChain).ProcessFilter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/filter.go:19\ngithub.com/emicklei/go-restful.(*Container).dispatch\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/container.go:282\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2012\nnet/http.(*ServeMux).ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2387\nnet/http.serverHandler.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2807\nnet/http.(*conn).serve\n\t/usr/local/go/src/net/http/server.go:1895\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1373\ncould not create CA of backend name ca-1\ngithub.com/Kong/kuma/pkg/core/managers/apis/mesh.EnsureEnabledCA\n\t/Users/marco/git/kuma/pkg/core/managers/apis/mesh/mesh_helpers.go:46\ngithub.com/Kong/kuma/pkg/core/managers/apis/mesh.(*meshManager).Update\n\t/Users/marco/git/kuma/pkg/core/managers/apis/mesh/mesh_manager.go:136\ngithub.com/Kong/kuma/pkg/core/resources/manager.(*customizableResourceManager).Update\n\t/Users/marco/git/kuma/pkg/core/resources/manager/customizable_manager.go:43\ngithub.com/Kong/kuma/pkg/api-server.(*resourceEndpoints).updateResource\n\t/Users/marco/git/kuma/pkg/api-server/resource_endpoints.go:144\ngithub.com/Kong/kuma/pkg/api-server.(*resourceEndpoints).createOrUpdateResource\n\t/Users/marco/git/kuma/pkg/api-server/resource_endpoints.go:128\ngithub.com/emicklei/go-restful.(*FilterChain).ProcessFilter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/filter.go:21\ngithub.com/emicklei/go-restful.CrossOriginResourceSharing.Filter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/cors_filter.go:40\ngithub.com/emicklei/go-restful.(*FilterChain).ProcessFilter\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/filter.go:19\ngithub.com/emicklei/go-restful.(*Container).dispatch\n\t/Users/marco/go/pkg/mod/github.com/emicklei/go-restful@v2.9.6+incompatible/container.go:282\nnet/http.HandlerFunc.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2012\nnet/http.(*ServeMux).ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2387\nnet/http.serverHandler.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2807\nnet/http.(*conn).serve\n\t/usr/local/go/src/net/http/server.go:1895\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1373"}

subnetmarco · 2020-06-01T16:53:37Z

This doesn't work - expecting a fix from @tharun208 to support days and years as the original issue described.

tharun208 requested a review from a team May 19, 2020 12:18

nickolaev approved these changes May 19, 2020

View reviewed changes

pkg/sds/server/server_test.go Show resolved Hide resolved

jakubdyszkiewicz requested changes May 19, 2020

View reviewed changes

tharun208 marked this pull request as draft May 20, 2020 15:47

tharun208 marked this pull request as ready for review May 21, 2020 20:12

tharun208 added 3 commits May 25, 2020 19:07

(feat-ca) Updated proto definitions for mesh

a0d5259

Updated ca plugins to parseDuration Updated tests Fix kumahq#743

(feat-ca) Updated proto definitions for builtinca

a3741a1

Updated builtinca manager to parseDuration Updated tests Fix kumahq#743

(feat-ca) Updated mesh validator

8608008

Fix kumahq#743

tharun208 force-pushed the certificate-expiration-time branch from d679f1e to 8608008 Compare May 25, 2020 13:41

tharun208 requested a review from jakubdyszkiewicz May 25, 2020 13:42

fixed conflicts

078950c

tharun208 requested a review from nickolaev May 27, 2020 13:40

nickolaev approved these changes May 27, 2020

View reviewed changes

jakubdyszkiewicz approved these changes May 28, 2020

View reviewed changes

jakubdyszkiewicz merged commit a6f2b30 into kumahq:master May 28, 2020

subnetmarco mentioned this pull request May 30, 2020

docs: mtls with new time format kumahq/kuma-website#209

Merged

tharun208 mentioned this pull request Jun 2, 2020

(fix ca):Updated Ca Certificate Expiration and Rotation by adding a c… #794

Merged

tharun208 deleted the certificate-expiration-time branch June 2, 2020 20:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(feat-ca) Ca rotation time to support months and year #750

(feat-ca) Ca rotation time to support months and year #750

tharun208 commented May 19, 2020

nickolaev left a comment

jakubdyszkiewicz left a comment •

edited

jakubdyszkiewicz left a comment •

edited

tharun208 commented May 20, 2020

jakubdyszkiewicz commented May 20, 2020

lobkovilya commented May 20, 2020

tharun208 commented May 28, 2020

nickolaev commented May 28, 2020

subnetmarco commented May 30, 2020 •

edited

subnetmarco commented Jun 1, 2020

(feat-ca) Ca rotation time to support months and year #750

(feat-ca) Ca rotation time to support months and year #750

Conversation

tharun208 commented May 19, 2020

Summary

Full changelog

Issues resolved

nickolaev left a comment

Choose a reason for hiding this comment

jakubdyszkiewicz left a comment • edited

Choose a reason for hiding this comment

jakubdyszkiewicz left a comment • edited

Choose a reason for hiding this comment

tharun208 commented May 20, 2020

jakubdyszkiewicz commented May 20, 2020

lobkovilya commented May 20, 2020

tharun208 commented May 28, 2020

nickolaev commented May 28, 2020

subnetmarco commented May 30, 2020 • edited

subnetmarco commented Jun 1, 2020

jakubdyszkiewicz left a comment •

edited

jakubdyszkiewicz left a comment •

edited

subnetmarco commented May 30, 2020 •

edited