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

Refactor CA to plugins #694

Merged
merged 10 commits into from
Apr 27, 2020
Merged

Refactor CA to plugins #694

merged 10 commits into from
Apr 27, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

Mesh model

Mesh model changed from

type: Mesh
name: default
mtls:
  enabled: true
  ca:
    builtin: {}
---
type: Mesh
name: default
mtls:
  enabled: true
  ca:
    provided: {}

to

type: Mesh
name: default
mtls:
  enabledBackend: ca-1 # enabled is gone, if this is present mTLS is enabled with backend of choice. If this is absent mTLS is off.
  backends:
  - name: ca-1
    type: builtin
  - name: ca-2
    type: provided
    config:
      cert:
        secret: sec-1
      key:
        secret: sec-2

Config can be anything (google.protobuf.Struct) therefore each plugin has to validate it's backend.

This also means that there might be multiple backends of the same type in one Mesh.

Provided CA

In case of provided, we no longer have to upload cert+key with kumactl manage ca provided. Instead you use Secret Management API to upload cert as a secret and reference it in the config.
Therefore kumactl manage ca provided (whole manage subcommand) is gone as well as webservice to handle it.
We no longer have to validate if there is only one cert since config implies that there is only one.

Data Source

I introduced a concept of DataSource. When defining a cert and key for provided

  - name: ca-2
    type: provided
    config:
      cert:
        secret: sec-1
      key:
        secret: sec-2

you can also provide the cert and key via file or with just inline bytes.

  - name: ca-2
    type: provided
    config:
      cert:
        inline: BASE64Bytes
      key:
        file: /opt/cert.pem

It was easy to implement and it's great for testing and may be also used on production if someone prefers to store certs in files.

Tests

Builtin CA was lacking in tests, so I added them. Tests for provided had to be rewritten.

I'm sorry about the volume of this PR. It was all tangled up and was really hard to split into multiple PRs and keep the functionality working.

@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team April 23, 2020 10:52
mtls = "builtin"
}
if mesh.MTLSEnabled() {
mtls = mesh.Spec.GetMtls().GetEnabledBackend()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe makes sense to write its type also, like builtin/ca-1? Because that change makes kumactl get meshes less informative

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question to @subnetmarco how should kumactl get meshes look like?
Right now given

type: Mesh
name: default
mtls:
  enabledBackend: ca-1
  backends:
  - name: ca-1
    type: builtin
  - name: ca-2
    type: provided
    config: ...

I print

NAME      mTLS  METRICS  LOGGING  TRACING
default   ca-1  off      off      off

If enabledBackend is not specified, I print off.

Ilya's argument is that it less information because previously we've seen the type (only the type since it was 1 CA per mesh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We came into conclusion with Marco that we will only display current backend with a format like you proposed which is type/name.
I will refactor also others in the same way when I unify the config for Tracing/Logging/Metrics.

pkg/core/ca/manager.go Outdated Show resolved Hide resolved
pkg/core/datasource/loader.go Outdated Show resolved Hide resolved
pkg/core/managers/apis/mesh/mesh_validator.go Show resolved Hide resolved
pkg/core/resources/apis/mesh/mesh_helpers.go Outdated Show resolved Hide resolved
pkg/plugins/ca/builtin/manager.go Show resolved Hide resolved
pkg/plugins/ca/builtin/manager.go Show resolved Hide resolved
pkg/core/managers/apis/mesh/mesh_helpers.go Show resolved Hide resolved
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.

None yet

2 participants