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

Sync access packages #147

Merged
merged 6 commits into from
Jun 5, 2018
Merged

Sync access packages #147

merged 6 commits into from
Jun 5, 2018

Conversation

ewoutp
Copy link
Contributor

@ewoutp ewoutp commented May 31, 2018

This PR aims to make the (dc2dc) replication easier and such that it no longer requires arangosync to be installed locally.

It does so by:

  • Simplify the settings in and ArangoDeploymentReplication (see docs)
  • Add a concept of an access package. This is a k8s resource file containing a secret for the TLS CA and a secret with a generated client authentication keyfile. See docs for details.
    The access package is created by editing the ArangoDeployment in the source cluster (add a name to the accessPackageSecretNames field). This yields the creation of a secret with that name.
    Extract the secret data (under accessPackage.yaml).
    Finally apply the accessPackage.yaml file in the destination cluster.

}

// String returns a string used to unique identify the authentication settings.
func (a Authentication) String() string {
return a.TLSAuthentication.String() + ":" + a.JWTSecret
return a.TLSAuthentication.String() + ":" + a.JWTSecret + ":" + a.Username + ":" + a.Password
Copy link

@matthewvon matthewvon Jun 5, 2018

Choose a reason for hiding this comment

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

I do not know where String() is used. But I wonder if making the password available via this function is required, and if not required, is it a bad thing. Could we accidentally expose a real user's password to a log file or such.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used to generate a key that matches the exact settings. So with exactly the same key you are allowed to share the same client object.

@@ -53,12 +76,23 @@ that is reachable from the Kubernetes cluster the `ArangoDeploymentReplication`

Specifying this setting and `spec.source.deploymentName` at the same time is not allowed.

### `spec.source.auth.jwtSecretName: string`
### `spec.source.auth.keyfileSecretName: string`

Choose a reason for hiding this comment

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

Is the user facing an upgrade issue if they previously used .jwtSecretName? Same question for .deploymentNamespace versus .endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was never part of a released version and was only just introduced in master, so I doubt it will cause problems.

used to authenticate the SyncMaster in the destination cluster with the SyncMaster in the
source cluster.
As a result, the destination Kubernetes cluster will have 2 additional `Secrets`. One containing a client authentication certificate
formatted ad a keyfile. Another containing the public key of the TLS CA certificate of the source cluster.

Choose a reason for hiding this comment

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

spelling or grammar or typo: "One containing a client authentication certificate formatted ad a keyfile." ad -> and ? If so, better wording would be "One contains a ...". Second sentence would then be "Another contains ..."

In response, a `Secret` is created in that Kubernetes cluster, with the given name, that contains a `accessPackage.yaml` data field
that contains a Kubernetes resource specification that can be inserted into the other Kubernetes cluster.

The process for creating and using an access package to authentication at the source cluster is as follows:

Choose a reason for hiding this comment

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

"package to authentication" -> "package for authentication"

@ewoutp ewoutp merged commit bb9b064 into master Jun 5, 2018
@ewoutp ewoutp deleted the feature/sync-access-pkg branch June 5, 2018 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants