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

Finish switching the helm plugin to connect error (2/2) #6323

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Finish switching the helm plugin to connect error (2/2) #6323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 19 additions & 21 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/getresourcerefs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ import (
"github.com/vmware-tanzu/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/resourcerefs/resourcerefstest"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -26,17 +24,17 @@ func TestGetInstalledPackageResourceRefs(t *testing.T) {
}

type testCase struct {
baseTestCase resourcerefstest.TestCase
request *corev1.GetInstalledPackageResourceRefsRequest
expectedResponse *corev1.GetInstalledPackageResourceRefsResponse
expectedStatusCode codes.Code
baseTestCase resourcerefstest.TestCase
request *corev1.GetInstalledPackageResourceRefsRequest
expectedResponse *corev1.GetInstalledPackageResourceRefsResponse
expectedErrorCode connect.Code
}

// newTestCase is a function to take an existing test-case
// (a so-called baseTestCase in pkg/resourcerefs module, which contains a LOT of useful data)
// and "enrich" it with some new fields to create a different kind of test case
// that tests server.GetInstalledPackageResourceRefs() func
newTestCase := func(tc int, identifier string, response bool, code codes.Code) testCase {
newTestCase := func(tc int, identifier string, response bool, code connect.Code) testCase {
newCase := testCase{
baseTestCase: resourcerefstest.TestCases1[tc],
request: &corev1.GetInstalledPackageResourceRefsRequest{
Expand All @@ -58,25 +56,25 @@ func TestGetInstalledPackageResourceRefs(t *testing.T) {
ResourceRefs: resourcerefstest.TestCases1[tc].ExpectedResourceRefs,
}
}
newCase.expectedStatusCode = code
newCase.expectedErrorCode = code
return newCase
}

testCases := []testCase{
newTestCase(0, "my-apache", true, codes.OK),
newTestCase(1, "my-apache", true, codes.OK),
newTestCase(2, "my-apache", true, codes.OK),
newTestCase(3, "my-apache", true, codes.OK),
newTestCase(4, "my-iis", false, codes.NotFound),
newTestCase(5, "my-apache", false, codes.Internal),
newTestCase(0, "my-apache", true, 0),
newTestCase(1, "my-apache", true, 0),
newTestCase(2, "my-apache", true, 0),
newTestCase(3, "my-apache", true, 0),
newTestCase(4, "my-iis", false, connect.CodeNotFound),
newTestCase(5, "my-apache", false, connect.CodeInternal),
// See https://github.com/vmware-tanzu/kubeapps/issues/632
newTestCase(6, "my-apache", true, codes.OK),
newTestCase(7, "my-apache", true, codes.OK),
newTestCase(8, "my-apache", true, codes.OK),
newTestCase(6, "my-apache", true, 0),
newTestCase(7, "my-apache", true, 0),
newTestCase(8, "my-apache", true, 0),
// See https://kubernetes.io/docs/reference/kubernetes-api/authorization-resources/role-v1/#RoleList
newTestCase(9, "my-apache", true, codes.OK),
newTestCase(9, "my-apache", true, 0),
// See https://kubernetes.io/docs/reference/kubernetes-api/authorization-resources/cluster-role-v1/#ClusterRoleList
newTestCase(10, "my-apache", true, codes.OK),
newTestCase(10, "my-apache", true, 0),
}

ignoredFields := cmpopts.IgnoreUnexported(
Expand Down Expand Up @@ -112,10 +110,10 @@ func TestGetInstalledPackageResourceRefs(t *testing.T) {

response, err := server.GetInstalledPackageResourceRefs(context.Background(), connect.NewRequest(tc.request))

if got, want := status.Code(err), tc.expectedStatusCode; got != want {
if got, want := connect.CodeOf(err), tc.expectedErrorCode; err != nil && got != want {
t.Fatalf("got: %+v, want: %+v, err: %+v", got, want, err)
}
if tc.expectedStatusCode != codes.OK {
if tc.expectedErrorCode != 0 {
return
}

Expand Down
4 changes: 1 addition & 3 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/connecterror"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/resources"
"github.com/vmware-tanzu/kubeapps/pkg/helm"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"
k8scorev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -654,7 +652,7 @@ func (s *Server) GetPackageRepositoryPermissions(ctx context.Context, request *c
cluster := request.Msg.GetContext().GetCluster()
namespace := request.Msg.GetContext().GetNamespace()
if cluster == "" && namespace != "" {
return nil, status.Errorf(codes.InvalidArgument, "cluster must be specified when namespace is present: %s", namespace)
return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("cluster must be specified when namespace is present: %s", namespace))
}
typedClient, err := s.clientGetter.Typed(request.Header(), cluster)
if err != nil {
Expand Down
12 changes: 5 additions & 7 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/connecterror"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
k8scorev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -114,13 +112,13 @@ func handleAuthSecretForUpdate(

// if we have both ref config and data config, it is an invalid mixed configuration
if (hasCaRef || hasAuthRef) && (hasCaData || hasAuthData) {
return nil, false, false, status.Errorf(codes.InvalidArgument, "Package repository cannot mix referenced secrets and user provided secret data")
return nil, false, false, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("Package repository cannot mix referenced secrets and user provided secret data"))
}

// check we cannot change mode (per design spec)
if secret != nil && (hasCaRef || hasCaData || hasAuthRef || hasAuthData) {
if isAuthSecretKubeappsManaged(repo, secret) != (hasAuthData || hasCaData) {
return nil, false, false, status.Errorf(codes.InvalidArgument, "Auth management mode cannot be changed")
return nil, false, false, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("Auth management mode cannot be changed"))
}
}

Expand Down Expand Up @@ -181,7 +179,7 @@ func handleImagesPullSecretForUpdate(
// check we are not changing mode
if secret != nil && (hasRef || hasData) {
if isImagesPullSecretKubeappsManaged(repo, secret) != hasData {
return nil, false, false, status.Errorf(codes.InvalidArgument, "Auth management mode cannot be changed")
return nil, false, false, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("Auth management mode cannot be changed"))
}
}

Expand Down Expand Up @@ -591,8 +589,8 @@ func validateUserManagedRepoSecret(

var secretRef string
if secretRefTls != "" && secretRefAuth != "" && secretRefTls != secretRefAuth {
return nil, status.Errorf(
codes.InvalidArgument, "TLS config secret and Auth secret must be the same")
return nil, connect.NewError(
connect.CodeInvalidArgument, fmt.Errorf("TLS config secret and Auth secret must be the same"))
} else if secretRefTls != "" {
secretRef = secretRefTls
} else if secretRefAuth != "" {
Expand Down
51 changes: 19 additions & 32 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
plugins "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/plugins/v1alpha1"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/plugins/helm/packages/v1alpha1"
"github.com/vmware-tanzu/kubeapps/pkg/helm"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"
authorizationv1 "k8s.io/api/authorization/v1"
apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -893,7 +891,7 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
request *corev1.GetPackageRepositorySummariesRequest
existingRepos []k8sruntime.Object
existingNamespaces []*apiv1.Namespace
expectedStatusCode codes.Code
expectedErrorCode connect.Code
expectedResponse *corev1.GetPackageRepositorySummariesResponse
reactors []*ClientReaction
}{
Expand All @@ -906,7 +904,6 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
&repo1b,
&repo2b,
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositorySummariesResponse{
PackageRepositorySummaries: []*corev1.PackageRepositorySummary{
repo1Summary,
Expand Down Expand Up @@ -975,7 +972,6 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
},
},
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositorySummariesResponse{
PackageRepositorySummaries: []*corev1.PackageRepositorySummary{
{
Expand All @@ -1000,7 +996,6 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
&repo1b,
&repo2b,
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositorySummariesResponse{
PackageRepositorySummaries: []*corev1.PackageRepositorySummary{
repo2Summary,
Expand All @@ -1015,7 +1010,6 @@ func TestGetPackageRepositorySummaries(t *testing.T) {
existingRepos: []k8sruntime.Object{
&repo3,
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositorySummariesResponse{
PackageRepositorySummaries: []*corev1.PackageRepositorySummary{
repo3Summary,
Expand Down Expand Up @@ -1045,12 +1039,12 @@ func TestGetPackageRepositorySummaries(t *testing.T) {

response, err := s.GetPackageRepositorySummaries(context.Background(), connect.NewRequest(tc.request))

if got, want := status.Code(err), tc.expectedStatusCode; got != want {
if got, want := connect.CodeOf(err), tc.expectedErrorCode; err != nil && got != want {
t.Fatalf("got: %+v, want: %+v, err: %+v", got, want, err)
}

// We don't need to check anything else for non-OK codes.
if tc.expectedStatusCode != codes.OK {
if tc.expectedErrorCode != 0 {
return
}

Expand Down Expand Up @@ -1114,20 +1108,19 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
request *corev1.GetPackageRepositoryDetailRequest
repositoryCustomizer func(repository *appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository
expectedResponse *corev1.GetPackageRepositoryDetailResponse
expectedStatusCode codes.Code
expectedErrorCode connect.Code
existingSecret *apiv1.Secret
}{
{
name: "not found",
request: buildRequest("ns-1", "foo"),
expectedStatusCode: codes.NotFound,
name: "not found",
request: buildRequest("ns-1", "foo"),
expectedErrorCode: connect.CodeNotFound,
},
{
name: "check ref",
request: buildRequest("ns-1", "repo-1"),
expectedResponse: buildResponse("ns-1", "repo-1", "helm",
"https://test-repo", "description 1", nil, nil, nil),
expectedStatusCode: codes.OK,
},
{
name: "check values with auth",
Expand All @@ -1145,8 +1138,7 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
},
nil,
nil),
existingSecret: newBasicAuthSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "baz-user", "zot-pwd"),
expectedStatusCode: codes.OK,
existingSecret: newBasicAuthSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "baz-user", "zot-pwd"),
},
{
name: "check values without auth",
Expand All @@ -1168,8 +1160,7 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
Variables: map[string]string{"$1": "value1"},
},
}),
existingSecret: newTlsSecret(helm.SecretNameForRepo("repo-4"), "ns-4", nil, nil, ca),
expectedStatusCode: codes.OK,
existingSecret: newTlsSecret(helm.SecretNameForRepo("repo-4"), "ns-4", nil, nil, ca),
},
{
name: "check values with imagesPullSecret",
Expand All @@ -1190,7 +1181,6 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
}),
existingSecret: newAuthDockerSecret(imagesPullSecretName("repo-5"), "ns-5",
dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")),
expectedStatusCode: codes.OK,
},
}

Expand All @@ -1216,9 +1206,9 @@ func TestGetPackageRepositoryDetail(t *testing.T) {
actualResponse, err := s.GetPackageRepositoryDetail(context.Background(), connect.NewRequest(tc.request))

// checks
if got, want := status.Code(err), tc.expectedStatusCode; got != want {
if got, want := connect.CodeOf(err), tc.expectedErrorCode; err != nil && got != want {
t.Fatalf("got error: %d, want: %d, err: %+v", got, want, err)
} else if got != codes.OK {
} else if got != 0 {
return
}

Expand Down Expand Up @@ -2109,11 +2099,11 @@ func TestDeletePackageRepository(t *testing.T) {
func TestGetPackageRepositoryPermissions(t *testing.T) {

testCases := []struct {
name string
request *corev1.GetPackageRepositoryPermissionsRequest
expectedStatusCode codes.Code
expectedResponse *corev1.GetPackageRepositoryPermissionsResponse
reactors []*ClientReaction
name string
request *corev1.GetPackageRepositoryPermissionsRequest
expectedErrorCode connect.Code
expectedResponse *corev1.GetPackageRepositoryPermissionsResponse
reactors []*ClientReaction
}{
{
name: "returns permissions for global package repositories",
Expand All @@ -2139,7 +2129,6 @@ func TestGetPackageRepositoryPermissions(t *testing.T) {
},
},
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositoryPermissionsResponse{
Permissions: []*corev1.PackageRepositoriesPermissions{
{
Expand Down Expand Up @@ -2169,7 +2158,6 @@ func TestGetPackageRepositoryPermissions(t *testing.T) {
},
},
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositoryPermissionsResponse{
Permissions: []*corev1.PackageRepositoriesPermissions{
{
Expand All @@ -2192,7 +2180,7 @@ func TestGetPackageRepositoryPermissions(t *testing.T) {
request: &corev1.GetPackageRepositoryPermissionsRequest{
Context: &corev1.Context{Namespace: "my-ns"},
},
expectedStatusCode: codes.InvalidArgument,
expectedErrorCode: connect.CodeInvalidArgument,
},
{
name: "returns permissions for namespaced package repositories",
Expand All @@ -2218,7 +2206,6 @@ func TestGetPackageRepositoryPermissions(t *testing.T) {
},
},
},
expectedStatusCode: codes.OK,
expectedResponse: &corev1.GetPackageRepositoryPermissionsResponse{
Permissions: []*corev1.PackageRepositoriesPermissions{
{
Expand Down Expand Up @@ -2251,12 +2238,12 @@ func TestGetPackageRepositoryPermissions(t *testing.T) {

response, err := s.GetPackageRepositoryPermissions(context.Background(), connect.NewRequest(tc.request))

if got, want := status.Code(err), tc.expectedStatusCode; got != want {
if got, want := connect.CodeOf(err), tc.expectedErrorCode; err != nil && got != want {
t.Fatalf("got: %+v, want: %+v, err: %+v", got, want, err)
}

// We don't need to check anything else for non-OK codes.
if tc.expectedStatusCode != codes.OK {
if tc.expectedErrorCode != 0 {
return
}

Expand Down
4 changes: 1 addition & 3 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import (

apprepov1alpha1 "github.com/vmware-tanzu/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
corev1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -85,7 +83,7 @@ func getValidator(appRepo *apprepov1alpha1.AppRepository) (HttpValidator, error)

func newRepositoryClient(appRepo *apprepov1alpha1.AppRepository, secret *corev1.Secret) (httpclient.Client, error) {
if cli, err := helm.InitNetClient(appRepo, secret, secret, nil); err != nil {
return nil, status.Errorf(codes.FailedPrecondition, "Unable to create HTTP client for repository: %v", err)
return nil, connect.NewError(connect.CodeFailedPrecondition, fmt.Errorf("Unable to create HTTP client for repository: %w", err))
} else {
return cli, nil
}
Expand Down
Loading