Skip to content

Commit

Permalink
remove gvrresolver use in the frontend (Azure#3087)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hawkowl
hawkowl authored Aug 24, 2023
1 parent f26f8f4 commit db0b249
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 66 deletions.
2 changes: 1 addition & 1 deletion pkg/frontend/admin_openshiftcluster_approvecsr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (f *frontend) _postAdminOpenShiftClusterApproveCSR(ctx context.Context, r *

csrName := r.URL.Query().Get("csrName")
if csrName != "" {
err := validateAdminKubernetesObjects(r.Method, &csrResource, "", csrName)
err := validateAdminKubernetesObjects(r.Method, csrResource, "", csrName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/frontend/admin_openshiftcluster_cordonnode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (f *frontend) _postAdminOpenShiftClusterCordonNode(ctx context.Context, r *

vmName := r.URL.Query().Get("vmName")
shouldCordon := strings.EqualFold(r.URL.Query().Get("shouldCordon"), "true")
err := validateAdminKubernetesObjects(r.Method, &nodeResource, "", vmName)
err := validateAdminKubernetesObjects(r.Method, nodeResource, "", vmName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/frontend/admin_openshiftcluster_drainnode.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (f *frontend) _postAdminOpenShiftClusterDrainNode(ctx context.Context, r *h
resType, resName, resGroupName := chi.URLParam(r, "resourceType"), chi.URLParam(r, "resourceName"), chi.URLParam(r, "resourceGroupName")

vmName := r.URL.Query().Get("vmName")
err := validateAdminKubernetesObjects(r.Method, &nodeResource, "", vmName)
err := validateAdminKubernetesObjects(r.Method, nodeResource, "", vmName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/frontend/admin_openshiftcluster_etcdrecovery.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (f *frontend) _postAdminOpenShiftClusterEtcdRecovery(ctx context.Context, r
return []byte{}, api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "", err.Error())
}

gvr, err := kubeActions.ResolveGVR("Etcd")
gvr, err := kubeActions.ResolveGVR("Etcd", "")
if err != nil {
return []byte{}, api.NewCloudError(http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "", err.Error())
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/frontend/admin_openshiftcluster_etcdrecovery_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestAdminEtcdRecovery(t *testing.T) {
ctx := context.Background()

resourceID := fmt.Sprintf("/subscriptions/%s/resourcegroups/resourceGroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/%s", mockSubID, resourceName)
gvk := &kschema.GroupVersionResource{
gvk := kschema.GroupVersionResource{
Group: "",
Version: "v1",
Resource: "Etcd",
Expand All @@ -89,7 +89,7 @@ func TestAdminEtcdRecovery(t *testing.T) {
wantError: "500: InternalServerError: : failed to parse resource",
doc: fakeRecoveryDoc(true, resourceID, resourceName),
mocks: func(ctx context.Context, ti *testInfra, k *mock_adminactions.MockKubeActions, log *logrus.Entry, env env.Interface, doc *api.OpenShiftClusterDocument, pods *corev1.PodList, etcdcli operatorv1client.EtcdInterface) {
k.EXPECT().ResolveGVR("Etcd").Times(1).Return(gvk, errors.New("failed to parse resource"))
k.EXPECT().ResolveGVR("Etcd", "").Times(1).Return(gvk, errors.New("failed to parse resource"))
},
},
{
Expand All @@ -99,7 +99,7 @@ func TestAdminEtcdRecovery(t *testing.T) {
wantError: "400: InvalidParameter: : The provided resource is invalid.",
doc: fakeRecoveryDoc(true, resourceID, resourceName),
mocks: func(ctx context.Context, ti *testInfra, k *mock_adminactions.MockKubeActions, log *logrus.Entry, env env.Interface, doc *api.OpenShiftClusterDocument, pods *corev1.PodList, etcdcli operatorv1client.EtcdInterface) {
k.EXPECT().ResolveGVR("Etcd").Times(1).Return(nil, nil)
k.EXPECT().ResolveGVR("Etcd", "").Times(1).Return(kschema.GroupVersionResource{}, nil)
},
},
{
Expand All @@ -109,7 +109,7 @@ func TestAdminEtcdRecovery(t *testing.T) {
wantError: "500: InternalServerError: : privateEndpointIP is empty",
doc: fakeRecoveryDoc(false, resourceID, resourceName),
mocks: func(ctx context.Context, ti *testInfra, k *mock_adminactions.MockKubeActions, log *logrus.Entry, env env.Interface, doc *api.OpenShiftClusterDocument, pods *corev1.PodList, etcdcli operatorv1client.EtcdInterface) {
k.EXPECT().ResolveGVR("Etcd").Times(1).Return(gvk, nil)
k.EXPECT().ResolveGVR("Etcd", "").Times(1).Return(gvk, nil)
},
},
{
Expand Down
6 changes: 3 additions & 3 deletions pkg/frontend/admin_openshiftcluster_kubernetesobjects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (f *frontend) _getAdminKubernetesObjects(ctx context.Context, r *http.Reque
return nil, err
}

gvr, err := k.ResolveGVR(groupKind)
gvr, err := k.ResolveGVR(groupKind, "")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -119,7 +119,7 @@ func (f *frontend) _deleteAdminKubernetesObjects(ctx context.Context, r *http.Re
return err
}

gvr, err := k.ResolveGVR(groupKind)
gvr, err := k.ResolveGVR(groupKind, "")
if err != nil {
return err
}
Expand Down Expand Up @@ -173,7 +173,7 @@ func (f *frontend) _postAdminKubernetesObjects(ctx context.Context, r *http.Requ
return err
}

gvr, err := k.ResolveGVR(obj.GetKind())
gvr, err := k.ResolveGVR(obj.GetKind(), "")
if err != nil {
return err
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/frontend/admin_openshiftcluster_kubernetesobjects_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
k.EXPECT().
KubeGet(gomock.Any(), tt.objKind, tt.objNamespace, tt.objName).
Return([]byte(`{"Kind": "test"}`), nil)
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "configmaps"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "configmaps"}, nil)
},
wantStatusCode: http.StatusOK,
wantResponse: []byte(`{"Kind": "test"}` + "\n"),
Expand All @@ -68,7 +68,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
k.EXPECT().
KubeList(gomock.Any(), tt.objKind, tt.objNamespace).
Return([]byte(`{"Kind": "test"}`), nil)
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "configmaps"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "configmaps"}, nil)
},
wantStatusCode: http.StatusOK,
wantResponse: []byte(`{"Kind": "test"}` + "\n"),
Expand All @@ -80,7 +80,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
objNamespace: "openshift",
objName: "config",
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objKind)
k.EXPECT().ResolveGVR(tt.objKind, "")
},
wantStatusCode: http.StatusBadRequest,
wantError: "400: InvalidParameter: : The provided resource is invalid.",
Expand All @@ -93,7 +93,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
objNamespace: "openshift",
objName: "config",
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "secrets"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "secrets"}, nil)
},
wantStatusCode: http.StatusForbidden,
wantError: "403: Forbidden: : Access to secrets is forbidden.",
Expand All @@ -109,7 +109,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
k.EXPECT().
KubeDelete(gomock.Any(), tt.objKind, tt.objNamespace, tt.objName, false, nil).
Return(nil)
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "configmaps"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "configmaps"}, nil)
},
wantStatusCode: http.StatusOK,
},
Expand All @@ -125,7 +125,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
k.EXPECT().
KubeDelete(gomock.Any(), tt.objKind, tt.objNamespace, tt.objName, true, nil).
Return(nil)
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "pods"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "pods"}, nil)
},
wantStatusCode: http.StatusOK,
},
Expand All @@ -149,7 +149,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
objNamespace: "openshift",
objName: "config",
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objKind)
k.EXPECT().ResolveGVR(tt.objKind, "")
},
wantStatusCode: http.StatusBadRequest,
wantError: "400: InvalidParameter: : The provided resource is invalid.",
Expand All @@ -161,7 +161,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
objKind: "this",
objNamespace: "openshift",
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objKind)
k.EXPECT().ResolveGVR(tt.objKind, "")
},
wantStatusCode: http.StatusBadRequest,
wantError: "400: InvalidParameter: : The provided resource is invalid.",
Expand All @@ -174,7 +174,7 @@ func TestAdminKubernetesObjectsGetAndDelete(t *testing.T) {
objNamespace: "openshift",
objName: "config",
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objKind).Return(&schema.GroupVersionResource{Resource: "secrets"}, nil)
k.EXPECT().ResolveGVR(tt.objKind, "").Return(schema.GroupVersionResource{Resource: "secrets"}, nil)
},
wantStatusCode: http.StatusForbidden,
wantError: "403: Forbidden: : Access to secrets is forbidden.",
Expand Down Expand Up @@ -270,7 +270,7 @@ func TestAdminPostKubernetesObjects(t *testing.T) {
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().KubeCreateOrUpdate(gomock.Any(), tt.objInBody).
Return(nil)
k.EXPECT().ResolveGVR(tt.objInBody.GetKind()).Return(&schema.GroupVersionResource{Resource: "configmaps"}, nil)
k.EXPECT().ResolveGVR(tt.objInBody.GetKind(), "").Return(schema.GroupVersionResource{Resource: "configmaps"}, nil)
},
wantStatusCode: http.StatusOK,
},
Expand All @@ -287,7 +287,7 @@ func TestAdminPostKubernetesObjects(t *testing.T) {
},
},
mocks: func(tt *test, k *mock_adminactions.MockKubeActions) {
k.EXPECT().ResolveGVR(tt.objInBody.GetKind()).Return(&schema.GroupVersionResource{Resource: "secrets"}, nil)
k.EXPECT().ResolveGVR(tt.objInBody.GetKind(), "").Return(schema.GroupVersionResource{Resource: "secrets"}, nil)
},
wantStatusCode: http.StatusForbidden,
wantError: "403: Forbidden: : Access to secrets is forbidden.",
Expand Down
37 changes: 19 additions & 18 deletions pkg/frontend/adminactions/kubeactions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ import (
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/env"
"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
"github.com/Azure/ARO-RP/pkg/util/restconfig"
)

Expand All @@ -31,7 +32,7 @@ type KubeActions interface {
KubeList(ctx context.Context, groupKind, namespace string) ([]byte, error)
KubeCreateOrUpdate(ctx context.Context, obj *unstructured.Unstructured) error
KubeDelete(ctx context.Context, groupKind, namespace, name string, force bool, propagationPolicy *metav1.DeletionPropagation) error
ResolveGVR(groupKind string) (*schema.GroupVersionResource, error)
ResolveGVR(groupKind string, optionalVersion string) (schema.GroupVersionResource, error)
CordonNode(ctx context.Context, nodeName string, unschedulable bool) error
DrainNode(ctx context.Context, nodeName string) error
ApproveCsr(ctx context.Context, csrName string) error
Expand All @@ -46,7 +47,7 @@ type kubeActions struct {
log *logrus.Entry
oc *api.OpenShiftCluster

gvrResolver dynamichelper.GVRResolver
mapper meta.RESTMapper

dyn dynamic.Interface
configcli configclient.Interface
Expand All @@ -60,7 +61,7 @@ func NewKubeActions(log *logrus.Entry, env env.Interface, oc *api.OpenShiftClust
return nil, err
}

gvrResolver, err := dynamichelper.NewGVRResolver(log, restConfig)
mapper, err := apiutil.NewDynamicRESTMapper(restConfig, apiutil.WithLazyDiscovery)
if err != nil {
return nil, err
}
Expand All @@ -84,7 +85,7 @@ func NewKubeActions(log *logrus.Entry, env env.Interface, oc *api.OpenShiftClust
log: log,
oc: oc,

gvrResolver: gvrResolver,
mapper: mapper,

dyn: dyn,
configcli: configcli,
Expand All @@ -98,17 +99,17 @@ func (k *kubeActions) KubeGetPodLogs(ctx context.Context, namespace, podName, co
return k.kubecli.CoreV1().Pods(namespace).GetLogs(podName, &opts).Do(ctx).Raw()
}

func (k *kubeActions) ResolveGVR(groupKind string) (*schema.GroupVersionResource, error) {
return k.gvrResolver.Resolve(groupKind, "")
func (k *kubeActions) ResolveGVR(groupKind string, optionalVersion string) (schema.GroupVersionResource, error) {
return k.mapper.ResourceFor(schema.ParseGroupResource(groupKind).WithVersion(optionalVersion))
}

func (k *kubeActions) KubeGet(ctx context.Context, groupKind, namespace, name string) ([]byte, error) {
gvr, err := k.gvrResolver.Resolve(groupKind, "")
gvr, err := k.ResolveGVR(groupKind, "")
if err != nil {
return nil, err
}

un, err := k.dyn.Resource(*gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
un, err := k.dyn.Resource(gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
if err != nil {
return nil, err
}
Expand All @@ -117,13 +118,13 @@ func (k *kubeActions) KubeGet(ctx context.Context, groupKind, namespace, name st
}

func (k *kubeActions) KubeList(ctx context.Context, groupKind, namespace string) ([]byte, error) {
gvr, err := k.gvrResolver.Resolve(groupKind, "")
gvr, err := k.ResolveGVR(groupKind, "")
if err != nil {
return nil, err
}

// protect RP memory by not reading in more than 1000 items
ul, err := k.dyn.Resource(*gvr).Namespace(namespace).List(ctx, metav1.ListOptions{Limit: 1000})
ul, err := k.dyn.Resource(gvr).Namespace(namespace).List(ctx, metav1.ListOptions{Limit: 1000})
if err != nil {
return nil, err
}
Expand All @@ -138,22 +139,22 @@ func (k *kubeActions) KubeList(ctx context.Context, groupKind, namespace string)
}

func (k *kubeActions) KubeCreateOrUpdate(ctx context.Context, o *unstructured.Unstructured) error {
gvr, err := k.gvrResolver.Resolve(o.GroupVersionKind().GroupKind().String(), o.GroupVersionKind().Version)
gvr, err := k.ResolveGVR(o.GroupVersionKind().GroupKind().String(), o.GroupVersionKind().Version)
if err != nil {
return err
}

_, err = k.dyn.Resource(*gvr).Namespace(o.GetNamespace()).Update(ctx, o, metav1.UpdateOptions{})
_, err = k.dyn.Resource(gvr).Namespace(o.GetNamespace()).Update(ctx, o, metav1.UpdateOptions{})
if !kerrors.IsNotFound(err) {
return err
}

_, err = k.dyn.Resource(*gvr).Namespace(o.GetNamespace()).Create(ctx, o, metav1.CreateOptions{})
_, err = k.dyn.Resource(gvr).Namespace(o.GetNamespace()).Create(ctx, o, metav1.CreateOptions{})
return err
}

func (k *kubeActions) KubeWatch(ctx context.Context, o *unstructured.Unstructured, labelKey string) (watch.Interface, error) {
gvr, err := k.gvrResolver.Resolve(o.GroupVersionKind().GroupKind().String(), o.GroupVersionKind().Version)
gvr, err := k.ResolveGVR(o.GroupVersionKind().GroupKind().String(), o.GroupVersionKind().Version)
if err != nil {
return nil, err
}
Expand All @@ -163,7 +164,7 @@ func (k *kubeActions) KubeWatch(ctx context.Context, o *unstructured.Unstructure
LabelSelector: o.GetLabels()[labelKey],
}

w, err := k.dyn.Resource(*gvr).Namespace(o.GetNamespace()).Watch(ctx, listOpts)
w, err := k.dyn.Resource(gvr).Namespace(o.GetNamespace()).Watch(ctx, listOpts)
if err != nil {
return nil, err
}
Expand All @@ -172,7 +173,7 @@ func (k *kubeActions) KubeWatch(ctx context.Context, o *unstructured.Unstructure
}

func (k *kubeActions) KubeDelete(ctx context.Context, groupKind, namespace, name string, force bool, propagationPolicy *metav1.DeletionPropagation) error {
gvr, err := k.gvrResolver.Resolve(groupKind, "")
gvr, err := k.ResolveGVR(groupKind, "")
if err != nil {
return err
}
Expand All @@ -186,5 +187,5 @@ func (k *kubeActions) KubeDelete(ctx context.Context, groupKind, namespace, name
resourceDeleteOptions.PropagationPolicy = propagationPolicy
}

return k.dyn.Resource(*gvr).Namespace(namespace).Delete(ctx, name, resourceDeleteOptions)
return k.dyn.Resource(gvr).Namespace(namespace).Delete(ctx, name, resourceDeleteOptions)
}
10 changes: 5 additions & 5 deletions pkg/frontend/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (f *frontend) validateOpenShiftUniqueKey(ctx context.Context, doc *api.Open
// prevent mischief
var rxKubernetesString = regexp.MustCompile(`(?i)^[-a-z0-9.]{0,255}$`)

func validatePermittedClusterwideObjects(gvr *schema.GroupVersionResource) bool {
func validatePermittedClusterwideObjects(gvr schema.GroupVersionResource) bool {
permittedGroups := map[string]bool{
"apiserver.openshift.io": true,
"aro.openshift.io": true,
Expand All @@ -100,8 +100,8 @@ func validatePermittedClusterwideObjects(gvr *schema.GroupVersionResource) bool
return permittedGroups[gvr.Group] || (groupHasException && allowedResources[gvr.Resource])
}

func validateAdminKubernetesObjectsNonCustomer(method string, gvr *schema.GroupVersionResource, namespace, name string) error {
if gvr == nil {
func validateAdminKubernetesObjectsNonCustomer(method string, gvr schema.GroupVersionResource, namespace, name string) error {
if gvr.Empty() {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The provided resource is invalid.")
}

Expand All @@ -116,8 +116,8 @@ func validateAdminKubernetesObjectsNonCustomer(method string, gvr *schema.GroupV
return validateAdminKubernetesObjects(method, gvr, namespace, name)
}

func validateAdminKubernetesObjects(method string, gvr *schema.GroupVersionResource, namespace, name string) error {
if gvr == nil {
func validateAdminKubernetesObjects(method string, gvr schema.GroupVersionResource, namespace, name string) error {
if gvr.Empty() {
return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "", "The provided resource is invalid.")
}

Expand Down
Loading

0 comments on commit db0b249

Please sign in to comment.