Skip to content

Commit 1d654f2

Browse files
authored
feat: added cluster validation for save/update (#2467)
* updated validation * updated error messaging * added unit test case * added integration test cases * review changes
1 parent 8f81e08 commit 1d654f2

File tree

4 files changed

+139
-3
lines changed

4 files changed

+139
-3
lines changed

pkg/cluster/ClusterService.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ import (
2121
"context"
2222
"fmt"
2323
"io/ioutil"
24+
"k8s.io/apimachinery/pkg/api/errors"
25+
"k8s.io/client-go/kubernetes"
2426
v12 "k8s.io/client-go/kubernetes/typed/core/v1"
27+
"k8s.io/client-go/rest"
28+
"net/url"
2529
"os"
2630
"time"
2731

@@ -35,6 +39,8 @@ import (
3539
"go.uber.org/zap"
3640
)
3741

42+
const DEFAULT_CLUSTER = "default_cluster"
43+
3844
type ClusterBean struct {
3945
Id int `json:"id,omitempty" validate:"number"`
4046
ClusterName string `json:"cluster_name,omitempty" validate:"required"`
@@ -133,6 +139,11 @@ func (impl *ClusterServiceImpl) GetClusterConfig(cluster *ClusterBean) (*util.Cl
133139
}
134140

135141
func (impl *ClusterServiceImpl) Save(parent context.Context, bean *ClusterBean, userId int32) (*ClusterBean, error) {
142+
//validating config
143+
err := impl.CheckIfConfigIsValid(bean)
144+
if err != nil {
145+
return nil, err
146+
}
136147
existingModel, err := impl.clusterRepository.FindOne(bean.ClusterName)
137148
if err != nil && err != pg.ErrNoRows {
138149
impl.logger.Error(err)
@@ -323,6 +334,11 @@ func (impl *ClusterServiceImpl) FindByIds(ids []int) ([]ClusterBean, error) {
323334
}
324335

325336
func (impl *ClusterServiceImpl) Update(ctx context.Context, bean *ClusterBean, userId int32) (*ClusterBean, error) {
337+
//validating config
338+
err := impl.CheckIfConfigIsValid(bean)
339+
if err != nil {
340+
return nil, err
341+
}
326342
model, err := impl.clusterRepository.FindById(bean.Id)
327343
if err != nil {
328344
impl.logger.Error(err)
@@ -478,3 +494,43 @@ func (impl ClusterServiceImpl) DeleteFromDb(bean *ClusterBean, userId int32) err
478494
}
479495
return nil
480496
}
497+
498+
func (impl ClusterServiceImpl) CheckIfConfigIsValid(cluster *ClusterBean) error {
499+
configMap := cluster.Config
500+
bearerToken := configMap["bearer_token"]
501+
var restConfig *rest.Config
502+
var err error
503+
if cluster.ClusterName == DEFAULT_CLUSTER && len(bearerToken) == 0 {
504+
restConfig, err = rest.InClusterConfig()
505+
if err != nil {
506+
impl.logger.Errorw("error in getting rest config for default cluster", "err", err)
507+
return err
508+
}
509+
} else {
510+
restConfig = &rest.Config{Host: cluster.ServerUrl, BearerToken: bearerToken, TLSClientConfig: rest.TLSClientConfig{Insecure: true}}
511+
}
512+
k8sClientSet, err := kubernetes.NewForConfig(restConfig)
513+
if err != nil {
514+
impl.logger.Errorw("error in getting client set by rest config", "err", err, "restConfig", restConfig)
515+
return err
516+
}
517+
//using livez path as healthz path is deprecated
518+
path := "/livez"
519+
response, err := k8sClientSet.Discovery().RESTClient().Get().AbsPath(path).DoRaw(context.Background())
520+
if err != nil {
521+
if _, ok := err.(*url.Error); ok {
522+
return fmt.Errorf("Incorrect server url : %v", err)
523+
} else if statusError, ok := err.(*errors.StatusError); ok {
524+
if statusError != nil {
525+
return fmt.Errorf("%s : %s", statusError.ErrStatus.Reason, statusError.ErrStatus.Message)
526+
} else {
527+
return fmt.Errorf("Validation failed : %v", err)
528+
}
529+
} else {
530+
return fmt.Errorf("Validation failed : %v", err)
531+
}
532+
} else if err == nil && string(response) != "ok" {
533+
return fmt.Errorf("Validation failed with response : %s", string(response))
534+
}
535+
return nil
536+
}

pkg/cluster/ClusterServiceExtended.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121
"go.uber.org/zap"
2222
)
2323

24-
//extends ClusterServiceImpl and enhances method of ClusterService with full mode specific errors
24+
// extends ClusterServiceImpl and enhances method of ClusterService with full mode specific errors
2525
type ClusterServiceImplExtended struct {
2626
environmentRepository repository.EnvironmentRepository
2727
grafanaClient grafana.GrafanaClient
@@ -126,12 +126,17 @@ func (impl *ClusterServiceImplExtended) FindAll() ([]*ClusterBean, error) {
126126
}
127127

128128
func (impl *ClusterServiceImplExtended) Update(ctx context.Context, bean *ClusterBean, userId int32) (*ClusterBean, error) {
129+
//validating config
130+
err := impl.CheckIfConfigIsValid(bean)
131+
if err != nil {
132+
return nil, err
133+
}
129134
isGitOpsConfigured, err1 := impl.gitOpsRepository.IsGitOpsConfigured()
130135
if err1 != nil {
131136
return nil, err1
132137
}
133138

134-
bean, err := impl.ClusterServiceImpl.Update(ctx, bean, userId)
139+
bean, err = impl.ClusterServiceImpl.Update(ctx, bean, userId)
135140
if err != nil {
136141
return nil, err
137142
}
@@ -299,6 +304,11 @@ func (impl *ClusterServiceImplExtended) CreateGrafanaDataSource(clusterBean *Clu
299304
}
300305

301306
func (impl *ClusterServiceImplExtended) Save(ctx context.Context, bean *ClusterBean, userId int32) (*ClusterBean, error) {
307+
//validating config
308+
err := impl.CheckIfConfigIsValid(bean)
309+
if err != nil {
310+
return nil, err
311+
}
302312
isGitOpsConfigured, err := impl.gitOpsRepository.IsGitOpsConfigured()
303313
if err != nil {
304314
return nil, err

pkg/cluster/ClusterService_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package cluster
2+
3+
import (
4+
"github.com/devtron-labs/devtron/client/k8s/informer"
5+
"github.com/devtron-labs/devtron/internal/util"
6+
"github.com/devtron-labs/devtron/pkg/cluster/repository"
7+
"go.uber.org/zap"
8+
"testing"
9+
)
10+
11+
func TestClusterServiceImpl_CheckIfConfigIsValid(t *testing.T) {
12+
t.SkipNow()
13+
type fields struct {
14+
clusterRepository repository.ClusterRepository
15+
logger *zap.SugaredLogger
16+
K8sUtil *util.K8sUtil
17+
K8sInformerFactory informer.K8sInformerFactory
18+
}
19+
type args struct {
20+
cluster *ClusterBean
21+
}
22+
tests := []struct {
23+
name string
24+
fields fields
25+
args args
26+
wantErr bool
27+
}{
28+
{
29+
//incorrect server config
30+
args: args{
31+
cluster: &ClusterBean{
32+
Id: 4,
33+
ServerUrl: "",
34+
Config: map[string]string{
35+
"bearer_token": "",
36+
},
37+
},
38+
},
39+
wantErr: true,
40+
},
41+
{
42+
//correct server config
43+
args: args{
44+
cluster: &ClusterBean{
45+
Id: 5,
46+
ServerUrl: "",
47+
Config: map[string]string{
48+
"bearer_token": "",
49+
},
50+
},
51+
},
52+
wantErr: false,
53+
},
54+
}
55+
56+
logger, _ := util.NewSugardLogger()
57+
for _, tt := range tests {
58+
t.Run(tt.name, func(t *testing.T) {
59+
impl := ClusterServiceImpl{
60+
clusterRepository: nil,
61+
logger: logger,
62+
K8sUtil: nil,
63+
K8sInformerFactory: nil,
64+
}
65+
if err := impl.CheckIfConfigIsValid(tt.args.cluster); (err != nil) != tt.wantErr {
66+
t.Errorf("ClusterServiceImpl.CheckIfConfigIsValid() error = %v, wantErr %v", err, tt.wantErr)
67+
}
68+
})
69+
}
70+
}

wire_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)