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

EVEREST-1709: backup rbac tests #904

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
226 changes: 226 additions & 0 deletions api/database_cluster_backup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
// everest
// Copyright (C) 2023 Percona LLC
//
// 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 api

import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"testing"

"github.com/golang-jwt/jwt/v5"
"github.com/labstack/echo/v4"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"

everestv1alpha1 "github.com/percona/everest-operator/api/v1alpha1"
"github.com/percona/everest/pkg/kubernetes"
"github.com/percona/everest/pkg/rbac"
"github.com/percona/everest/pkg/rbac/mocks"
"github.com/percona/everest/pkg/session"
)

type enforceStrings struct{ subject, resource, action, object string }

func TestListDBCluster_permissions(t *testing.T) {
t.Parallel()

err := everestv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme)
require.NoError(t, err)
metav1.AddToGroupVersion(scheme.Scheme, everestv1alpha1.GroupVersion)

l := zap.NewNop().Sugar()

testCases := []struct {
name string
apiCall func(echo.Context, *EverestServer) error
body any
httpMethod string
kubeClient func() kubernetes.KubernetesConnector
proxyResponse any

expectedEnforce []enforceStrings
}{
{
name: "List DB cluster backups",
httpMethod: http.MethodGet,
apiCall: func(ctx echo.Context, everest *EverestServer) error {
return everest.ListDatabaseClusterBackups(ctx, "ns", "my-name")
},
proxyResponse: &everestv1alpha1.DatabaseClusterBackupList{
TypeMeta: metav1.TypeMeta{
Kind: "database-cluster-backup-list",
},
Items: []everestv1alpha1.DatabaseClusterBackup{
{
ObjectMeta: metav1.ObjectMeta{
Name: "name1",
Namespace: "ns",
},
Spec: everestv1alpha1.DatabaseClusterBackupSpec{
DBClusterName: "db-cluster-name",
},
},
},
},

expectedEnforce: []enforceStrings{
{"alice", rbac.ResourceBackupStorages, rbac.ActionRead, "ns/name1"},
Comment on lines +78 to +86
Copy link
Collaborator

@recharte recharte Dec 6, 2024

Choose a reason for hiding this comment

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

I think these tests just helped us find a bug 😅
The enforce for ResourceBackupStorages shouldn't be ns/name1. Instead, it should be ns/backup-storage-name that is part of the Spec like so:

Suggested change
Spec: everestv1alpha1.DatabaseClusterBackupSpec{
DBClusterName: "db-cluster-name",
},
},
},
},
expectedEnforce: []enforceStrings{
{"alice", rbac.ResourceBackupStorages, rbac.ActionRead, "ns/name1"},
Spec: everestv1alpha1.DatabaseClusterBackupSpec{
DBClusterName: "db-cluster-name",
BackupStorageName: "backup-storage-name",
},
},
},
},
expectedEnforce: []enforceStrings{
{"alice", rbac.ResourceBackupStorages, rbac.ActionRead, "ns/backup-storage-name"},

The bug is here. It should be dbbackup.Spec.BackupStorageName instead of dbbackup.GetName()

Copy link
Collaborator

Choose a reason for hiding this comment

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

The other functions have the same bug 🙈

{"alice", rbac.ResourceDatabaseClusterBackups, rbac.ActionRead, "ns/db-cluster-name"},
},
},
{
name: "Get DB cluster backup",
httpMethod: http.MethodGet,
apiCall: func(ctx echo.Context, everest *EverestServer) error {
return everest.GetDatabaseClusterBackup(ctx, "ns", "my-name")
},
kubeClient: func() kubernetes.KubernetesConnector {
k := kubernetes.NewMockKubernetesConnector(t)
k.EXPECT().GetDatabaseClusterBackup(mock.Anything, mock.Anything, mock.Anything).Return(
&everestv1alpha1.DatabaseClusterBackup{
TypeMeta: metav1.TypeMeta{
Kind: "database-cluster-backup",
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
Name: "name1",
},
Spec: everestv1alpha1.DatabaseClusterBackupSpec{
DBClusterName: "db-cluster-name",
},
}, nil,
)

return k
},

expectedEnforce: []enforceStrings{
{"alice", rbac.ResourceBackupStorages, rbac.ActionRead, "ns/name1"},
{"alice", rbac.ResourceDatabaseClusterBackups, rbac.ActionRead, "ns/db-cluster-name"},
},
},
{
name: "Create DB cluster backup",
httpMethod: http.MethodPost,
body: &DatabaseClusterBackup{
Spec: &struct {
BackupStorageName string "json:\"backupStorageName\""
DbClusterName string "json:\"dbClusterName\""

Check failure on line 127 in api/database_cluster_backup_test.go

View workflow job for this annotation

GitHub Actions / Check (1.23.x, false)

ST1003: struct field DbClusterName should be DBClusterName (stylecheck)
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
ST1003: struct field DbClusterName should be DBClusterName (stylecheck)

}{
BackupStorageName: "name1",
DbClusterName: "db-cluster-name",
},
},
apiCall: func(ctx echo.Context, everest *EverestServer) error {
return everest.CreateDatabaseClusterBackup(ctx, "ns")
},
proxyResponse: &everestv1alpha1.DatabaseClusterBackup{},
kubeClient: func() kubernetes.KubernetesConnector {
k := kubernetes.NewMockKubernetesConnector(t)
k.EXPECT().GetDatabaseCluster(mock.Anything, mock.Anything, mock.Anything).Return(
&everestv1alpha1.DatabaseCluster{
Spec: everestv1alpha1.DatabaseClusterSpec{
Engine: everestv1alpha1.Engine{
Type: everestv1alpha1.DatabaseEnginePXC,
},
},
}, nil,
)
k.EXPECT().ListDatabaseClusterBackups(mock.Anything, mock.Anything, mock.Anything).Return(
&everestv1alpha1.DatabaseClusterBackupList{
Items: []everestv1alpha1.DatabaseClusterBackup{},
}, nil,
)

return k
},

expectedEnforce: []enforceStrings{
{"alice", rbac.ResourceBackupStorages, rbac.ActionRead, "ns/name1"},
{"alice", rbac.ResourceDatabaseClusterBackups, rbac.ActionCreate, "ns/db-cluster-name"},
},
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

enforcer := &mocks.IEnforcer{}
kp := newMockK8sProxier(t)
if tt.proxyResponse != nil {
kp.EXPECT().proxyKubernetes(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Run(func(
ctx echo.Context, namespace, kind, name string,

Check failure on line 173 in api/database_cluster_backup_test.go

View workflow job for this annotation

GitHub Actions / Check (1.23.x, false)

unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
unused-parameter: parameter 'ctx' seems to be unused, consider removing or renaming it as _ (revive)

respTransformers ...apiResponseTransformerFn,
) {
b, err := json.Marshal(tt.proxyResponse)
require.NoError(t, err)

resp := &http.Response{
Body: io.NopCloser(bytes.NewReader(b)),
Header: make(http.Header),
}

modify := modifiersFn(l, respTransformers...)

Check failure on line 184 in api/database_cluster_backup_test.go

View workflow job for this annotation

GitHub Actions / Check (1.23.x, false)

response body must be closed (bodyclose)
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
response body must be closed (bodyclose)

err = modify(resp)
require.NoError(t, err)
}).Return(nil).Once()
}

for _, e := range tt.expectedEnforce {
enforcer.EXPECT().Enforce(e.subject, e.resource, e.action, e.object).Return(true, nil)
}

everest := &EverestServer{
rbacEnforcer: enforcer,
l: l,
k8sProxier: kp,
}
if tt.kubeClient != nil {
everest.kubeClient = tt.kubeClient()
}

var body io.Reader
if tt.body != nil {
b, err := json.Marshal(tt.body)
require.NoError(t, err)
body = bytes.NewReader(b)
}

req, err := http.NewRequest(tt.httpMethod, "/", body)

Check failure on line 210 in api/database_cluster_backup_test.go

View workflow job for this annotation

GitHub Actions / Check (1.23.x, false)

should rewrite http.NewRequestWithContext or add (*Request).WithContext (noctx)
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci-lint] reported by reviewdog 🐶
should rewrite http.NewRequestWithContext or add (*Request).WithContext (noctx)

require.NoError(t, err)
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
rec := httptest.NewRecorder()
e := echo.New()
c := e.NewContext(req, rec)
claims := make(jwt.MapClaims)
claims["sub"] = "alice"
claims["iss"] = session.SessionManagerClaimsIssuer

c.Set("user", &jwt.Token{Claims: claims})

err = tt.apiCall(c, everest)
require.NoError(t, err)
})
}
}
13 changes: 9 additions & 4 deletions api/everest.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ type EverestServer struct {
config *config.EverestConfig
l *zap.SugaredLogger
echo *echo.Echo
kubeClient *kubernetes.Kubernetes
k8sProxier k8sProxier
kubeClient kubernetes.KubernetesConnector
sessionMgr *session.Manager
attemptsStore *RateLimiterMemoryStore
rbacEnforcer casbin.IEnforcer
Expand All @@ -80,9 +81,13 @@ func NewEverestServer(ctx context.Context, c *config.EverestConfig, l *zap.Sugar
return nil, errors.Join(err, errors.New("failed to create session manager"))
}
e := &EverestServer{
config: c,
l: l,
echo: echoServer,
config: c,
l: l,
echo: echoServer,
k8sProxier: &k8sProxy{
kubeClient: kubeClient,
l: l,
},
kubeClient: kubeClient,
sessionMgr: sessMgr,
attemptsStore: store,
Expand Down
18 changes: 18 additions & 0 deletions api/gen.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// everest
// Copyright (C) 2023 Percona LLC
//
// 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 api

//go:generate ../bin/mockery --name=k8sProxier --case=snake --inpackage --with-expecter=true
2 changes: 1 addition & 1 deletion api/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (e *EverestServer) GetKubernetesClusterResources(ctx echo.Context) error {
}

func (e *EverestServer) calculateClusterResources(
ctx echo.Context, kubeClient *kubernetes.Kubernetes, clusterType kubernetes.ClusterType,
ctx echo.Context, kubeClient kubernetes.KubernetesConnector, clusterType kubernetes.ClusterType,
volumes *corev1.PersistentVolumeList,
) (*KubernetesClusterResources, error) {
allCPUMillis, allMemoryBytes, allDiskBytes, err := kubeClient.GetAllClusterResources(
Expand Down
100 changes: 100 additions & 0 deletions api/mock_k8s_proxier.go

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

Loading
Loading