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.

Sign up for GitHub

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

Jump to bottom

chore: improve error logs in server/cluster/cluser.go #20711

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+35 −34
Open

chore: improve error logs in server/cluster/cluser.go #20711

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1acd7c1
fix: wrap error objects to include context in error handling
SuminSSon Nov 8, 2024
0c51ed9
fix: apply gofumpt formatting
SuminSSon Nov 8, 2024
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
69 changes: 35 additions & 34 deletions server/cluster/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cluster

import (
"context"
"fmt"
"net/url"
"time"

Expand Down Expand Up @@ -53,14 +54,14 @@ func CreateClusterRBACObject(project string, server string) string {
func (s *Server) List(ctx context.Context, q *cluster.ClusterQuery) (*appv1.ClusterList, error) {
clusterList, err := s.db.ListClusters(ctx)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to list clusters: %w", err)
}

filteredItems := clusterList.Items

// Filter clusters by id
if filteredItems, err = filterClustersById(filteredItems, q.Id); err != nil {
return nil, err
return nil, fmt.Errorf("error filtering clusters by id: %w", err)
}

// Filter clusters by name
Expand All @@ -80,7 +81,7 @@ func (s *Server) List(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Clus
return nil
})
if err != nil {
return nil, err
return nil, fmt.Errorf("error running async cluster responses: %w", err)
}

cl := *clusterList
Expand All @@ -102,7 +103,7 @@ func filterClustersById(clusters []appv1.Cluster, id *cluster.ClusterID) ([]appv
case "name_escaped":
nameUnescaped, err := url.QueryUnescape(id.Value)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to unescape cluster name: %w", err)
}
items = filterClustersByName(clusters, nameUnescaped)
default:
Expand Down Expand Up @@ -143,17 +144,17 @@ func filterClustersByServer(clusters []appv1.Cluster, server string) []appv1.Clu
// Create creates a cluster
func (s *Server) Create(ctx context.Context, q *cluster.ClusterCreateRequest) (*appv1.Cluster, error) {
if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionCreate, CreateClusterRBACObject(q.Cluster.Project, q.Cluster.Server)); err != nil {
return nil, err
return nil, fmt.Errorf("permission denied while creating cluster: %w", err)
}
c := q.Cluster
clusterRESTConfig, err := c.RESTConfig()
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting REST config: %w", err)
}

serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting server version: %w", err)
}

clust, err := s.db.CreateCluster(ctx, c)
Expand All @@ -173,7 +174,7 @@ func (s *Server) Create(ctx context.Context, q *cluster.ClusterCreateRequest) (*
return nil, status.Error(codes.InvalidArgument, argo.GenerateSpecIsDifferentErrorMessage("cluster", existing, c))
}
} else {
return nil, err
return nil, fmt.Errorf("error creating cluster: %w", err)
}
}

Expand All @@ -185,7 +186,7 @@ func (s *Server) Create(ctx context.Context, q *cluster.ClusterCreateRequest) (*
},
})
if err != nil {
return nil, err
return nil, fmt.Errorf("error setting cluster info in cache: %w", err)
}
return s.toAPIResponse(clust), err
}
Expand All @@ -194,7 +195,7 @@ func (s *Server) Create(ctx context.Context, q *cluster.ClusterCreateRequest) (*
func (s *Server) Get(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Cluster, error) {
c, err := s.getClusterAndVerifyAccess(ctx, q, rbacpolicy.ActionGet)
if err != nil {
return nil, err
return nil, fmt.Errorf("error verifying access to update cluster: %w", err)
}

return s.toAPIResponse(c), nil
Expand All @@ -211,7 +212,7 @@ func (s *Server) getClusterWith403IfNotExist(ctx context.Context, q *cluster.Clu
func (s *Server) getClusterAndVerifyAccess(ctx context.Context, q *cluster.ClusterQuery, action string) (*appv1.Cluster, error) {
c, err := s.getClusterWith403IfNotExist(ctx, q)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get cluster with permissions check: %w", err)
}

// verify that user can do the specified action inside project where cluster is located
Expand All @@ -232,7 +233,7 @@ func (s *Server) getCluster(ctx context.Context, q *cluster.ClusterQuery) (*appv
} else if q.Id.Type == "name_escaped" {
nameUnescaped, err := url.QueryUnescape(q.Id.Value)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to unescape cluster name: %w", err)
}
q.Name = nameUnescaped
} else {
Expand All @@ -243,7 +244,7 @@ func (s *Server) getCluster(ctx context.Context, q *cluster.ClusterQuery) (*appv
if q.Server != "" {
c, err := s.db.GetCluster(ctx, q.Server)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get cluster by server: %w", err)
}
return c, nil
}
Expand All @@ -253,7 +254,7 @@ func (s *Server) getCluster(ctx context.Context, q *cluster.ClusterQuery) (*appv
if q.Name != "" {
clusterList, err := s.db.ListClusters(ctx)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to list clusters: %w", err)
}
for _, c := range clusterList.Items {
if c.Name == q.Name {
Expand Down Expand Up @@ -300,7 +301,7 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
Id: q.Id,
}, rbacpolicy.ActionUpdate)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to verify access for updating cluster: %w", err)
}

if len(q.UpdatedFields) == 0 || sets.NewString(q.UpdatedFields...).Has("project") {
Expand All @@ -320,18 +321,18 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
}
clusterRESTConfig, err := q.Cluster.RESTConfig()
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get REST config for cluster: %w", err)
}

// Test the token we just created before persisting it
serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get server version: %w", err)
}

clust, err := s.db.UpdateCluster(ctx, q.Cluster)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to update cluster in database: %w", err)
}
err = s.cache.SetClusterInfo(clust.Server, &appv1.ClusterInfo{
ServerVersion: serverVersion,
Expand All @@ -341,7 +342,7 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
},
})
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to set cluster info in cache: %w", err)
}
return s.toAPIResponse(clust), nil
}
Expand All @@ -350,7 +351,7 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
func (s *Server) Delete(ctx context.Context, q *cluster.ClusterQuery) (*cluster.ClusterResponse, error) {
c, err := s.getClusterWith403IfNotExist(ctx, q)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get cluster with permissions check: %w", err)
}

if q.Name != "" {
Expand All @@ -361,12 +362,12 @@ func (s *Server) Delete(ctx context.Context, q *cluster.ClusterQuery) (*cluster.
}
for _, server := range servers {
if err := enforceAndDelete(s, ctx, server, c.Project); err != nil {
return nil, err
return nil, fmt.Errorf("failed to enforce and delete cluster server: %w", err)
}
}
} else {
if err := enforceAndDelete(s, ctx, q.Server, c.Project); err != nil {
return nil, err
return nil, fmt.Errorf("failed to enforce and delete cluster server: %w", err)
}
}

Expand All @@ -388,7 +389,7 @@ func enforceAndDelete(s *Server, ctx context.Context, server, project string) er
func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*cluster.ClusterResponse, error) {
clust, err := s.getClusterWith403IfNotExist(ctx, q)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get cluster with permissions check: %w", err)
}

var servers []string
Expand Down Expand Up @@ -417,23 +418,23 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus
logCtx.Info("Rotating auth")
restCfg, err := clust.RESTConfig()
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get REST config for cluster: %w", err)
}
if restCfg.BearerToken == "" {
return nil, status.Errorf(codes.InvalidArgument, "Cluster '%s' does not use bearer token authentication", server)
}

claims, err := clusterauth.ParseServiceAccountToken(restCfg.BearerToken)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to parse service account token: %w", err)
}
kubeclientset, err := kubernetes.NewForConfig(restCfg)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to create Kubernetes clientset: %w", err)
}
newSecret, err := clusterauth.GenerateNewClusterManagerSecret(kubeclientset, claims)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to generate new cluster manager secret: %w", err)
}
// we are using token auth, make sure we don't store client-cert information
clust.Config.KeyData = nil
Expand All @@ -442,16 +443,16 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus

clusterRESTConfig, err := clust.RESTConfig()
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get REST config for cluster: %w", err)
}
// Test the token we just created before persisting it
serverVersion, err := s.kubectl.GetServerVersion(clusterRESTConfig)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get server version: %w", err)
}
_, err = s.db.UpdateCluster(ctx, clust)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to update cluster in database: %w", err)
}
err = s.cache.SetClusterInfo(clust.Server, &appv1.ClusterInfo{
ServerVersion: serverVersion,
Expand All @@ -461,11 +462,11 @@ func (s *Server) RotateAuth(ctx context.Context, q *cluster.ClusterQuery) (*clus
},
})
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to set cluster info in cache: %w", err)
}
err = clusterauth.RotateServiceAccountSecrets(kubeclientset, claims, newSecret)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to rotate service account secrets: %w", err)
}
logCtx.Infof("Rotated auth (old: %s, new: %s)", claims.SecretName, newSecret.Name)
}
Expand Down Expand Up @@ -498,13 +499,13 @@ func (s *Server) toAPIResponse(clust *appv1.Cluster) *appv1.Cluster {
func (s *Server) InvalidateCache(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Cluster, error) {
cls, err := s.getClusterAndVerifyAccess(ctx, q, rbacpolicy.ActionUpdate)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to verify access for cluster: %w", err)
}
now := v1.Now()
cls.RefreshRequestedAt = &now
cls, err = s.db.UpdateCluster(ctx, cls)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to update cluster in database: %w", err)
}
return s.toAPIResponse(cls), nil
}
Loading