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

Kopia Integration: Unified Repository Provider - Repo Password #5167

Merged
merged 1 commit into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/5167-lyndon
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add changes for Kopia Integration: Unified Repository Provider - Repo Password
24 changes: 24 additions & 0 deletions internal/credentials/getter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
Copyright the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package credentials

// CredentialGetter is a collection of interfaces for interacting with credentials
// that are stored in different targets
type CredentialGetter struct {
FromFile FileStore
FromSecret SecretStore
}
49 changes: 49 additions & 0 deletions internal/credentials/mocks/FileStore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions internal/credentials/mocks/SecretStore.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 56 additions & 0 deletions internal/credentials/secret_store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
Copyright the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package credentials

import (
"github.com/pkg/errors"
corev1api "k8s.io/api/core/v1"
kbclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/vmware-tanzu/velero/pkg/util/kube"
)

// SecretStore defines operations for interacting with credentials
// that are stored in Secret.
type SecretStore interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems it isn't a store? It's just a secret getter and stores nothing. Can we call kubeClient.Get() to get the secret directly without defining this interface?

If it's needed, I have the same comment with @qiuming-best , the function name Buffer confuses me

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 add this SecretStore in parallel with the existing FileStore, the FileStore gets the password from Secret then save to a temp file; SecretStore gets the password from Secret and then return a stream buffer.

We don't call kubeClient.Get() directly for below purpose:

  • We may get the password/credential keys from different sources, for example, from a temp file, from memory, from KMS, so we wrap the interfaces for all the sources in the CredentialGetter structure.
  • At present, in Unified Repo, we are using repo password got from memory, cloud provider credentials from a temp file
  • In future, we may add a new interface called something like KmsStore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the method name of SecretStore, changed from Buffer to Get

// Get returns the secret key defined by the given selector
Get(selector *corev1api.SecretKeySelector) (string, error)
}

type namespacedSecretStore struct {
client kbclient.Client
namespace string
}

// NewNamespacedSecretStore returns a SecretStore which can interact with credentials
// for the given namespace.
func NewNamespacedSecretStore(client kbclient.Client, namespace string) (SecretStore, error) {
return &namespacedSecretStore{
client: client,
namespace: namespace,
}, nil
}

// Buffer returns the secret key defined by the given selector.
func (n *namespacedSecretStore) Get(selector *corev1api.SecretKeySelector) (string, error) {
creds, err := kube.GetSecretKey(n.client, n.namespace, selector)
if err != nil {
return "", errors.Wrap(err, "unable to get key for secret")
}

return string(creds), nil
}
3 changes: 2 additions & 1 deletion pkg/cmd/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import (
"github.com/vmware-tanzu/velero/internal/storage"
"github.com/vmware-tanzu/velero/internal/util/managercontroller"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
)

const (
Expand Down Expand Up @@ -519,7 +520,7 @@ func (s *server) initRestic() error {
}

// ensure the repo key secret is set up
if err := restic.EnsureCommonRepositoryKey(s.kubeClient.CoreV1(), s.namespace); err != nil {
if err := repokey.EnsureCommonRepositoryKey(s.kubeClient.CoreV1(), s.namespace); err != nil {
return err
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/pod_volume_backup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/vmware-tanzu/velero/internal/credentials"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
"github.com/vmware-tanzu/velero/pkg/metrics"
repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/util/filesystem"
"github.com/vmware-tanzu/velero/pkg/util/kube"
Expand Down Expand Up @@ -324,7 +325,7 @@ func (r *PodVolumeBackupReconciler) buildResticCommand(ctx context.Context, log
log.WithField("path", path).Debugf("Found path matching glob")

// Temporary credentials.
details.credsFile, err = r.CredsFileStore.Path(restic.RepoKeySelector())
details.credsFile, err = r.CredsFileStore.Path(repokey.RepoKeySelector())
if err != nil {
return nil, errors.Wrap(err, "creating temporary Restic credentials file")
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/pod_volume_restore_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (

"github.com/vmware-tanzu/velero/internal/credentials"
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
repokey "github.com/vmware-tanzu/velero/pkg/repository/keys"
"github.com/vmware-tanzu/velero/pkg/restic"
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
"github.com/vmware-tanzu/velero/pkg/util/filesystem"
Expand Down Expand Up @@ -241,7 +242,7 @@ func (c *PodVolumeRestoreReconciler) processRestore(ctx context.Context, req *ve
return errors.Wrap(err, "error identifying path of volume")
}

credsFile, err := c.credentialsFileStore.Path(restic.RepoKeySelector())
credsFile, err := c.credentialsFileStore.Path(repokey.RepoKeySelector())
if err != nil {
return errors.Wrap(err, "error creating temp restic credentials file")
}
Expand Down
75 changes: 75 additions & 0 deletions pkg/repository/keys/keys.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
Copyright the Velero contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package keys

import (
"context"

"github.com/pkg/errors"
corev1api "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"

"github.com/vmware-tanzu/velero/pkg/builder"
)

const (
credentialsSecretName = "velero-restic-credentials"
credentialsKey = "repository-password"

encryptionKey = "static-passw0rd"
Copy link
Contributor

Choose a reason for hiding this comment

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

which means that our kopia repo paasword is plain text and invariable? and which may lead to some security issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the existing code and file, this keys.go is moved from Restic package to the repository package.
As the agreement, in v1.10, we will keep the existing behaviors for Kopia integration.

In future, as you can see, the provider construct accepts a CredentialGetter, in which, we can add whatever implementation to get a password. For example, we may get repo password from KMS in future, then we can add something like KmsStore interface into it.

)

func EnsureCommonRepositoryKey(secretClient corev1client.SecretsGetter, namespace string) error {
_, err := secretClient.Secrets(namespace).Get(context.TODO(), credentialsSecretName, metav1.GetOptions{})
if err != nil && !apierrors.IsNotFound(err) {
return errors.WithStack(err)
}
if err == nil {
return nil
}

// if we got here, we got an IsNotFound error, so we need to create the key

secret := &corev1api.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: credentialsSecretName,
},
Type: corev1api.SecretTypeOpaque,
Data: map[string][]byte{
credentialsKey: []byte(encryptionKey),
},
}

if _, err = secretClient.Secrets(namespace).Create(context.TODO(), secret, metav1.CreateOptions{}); err != nil {
return errors.Wrapf(err, "error creating %s secret", credentialsSecretName)
}

return nil
}

// RepoKeySelector returns the SecretKeySelector which can be used to fetch
// the restic repository key.
func RepoKeySelector() *corev1api.SecretKeySelector {
// For now, all restic repos share the same key so we don't need the repoName to fetch it.
// When we move to full-backup encryption, we'll likely have a separate key per restic repo
// (all within the Velero server's namespace) so RepoKeySelector will need to select the key
// for that repo.
return builder.ForSecretKeySelector(credentialsSecretName, credentialsKey).Result()
}
30 changes: 30 additions & 0 deletions pkg/repository/keys/keys_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
Copyright the Velero contributors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package keys

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestRepoKeySelector(t *testing.T) {
selector := RepoKeySelector()

require.Equal(t, credentialsSecretName, selector.Name)
require.Equal(t, credentialsKey, selector.Key)
}
Loading