diff --git a/pkg/portal/cluster.go b/pkg/portal/cluster.go index d2840bb94..00d993a0f 100644 --- a/pkg/portal/cluster.go +++ b/pkg/portal/cluster.go @@ -185,13 +185,13 @@ func (p *portal) machines(w http.ResponseWriter, r *http.Request) { func (p *portal) VMAllocationStatus(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - fetcher, err := p.makeFetcher(ctx, r) + azurefetcher, err := p.makeAzureFetcher(ctx, r) if err != nil { p.internalServerError(w, err) return } - machineVMAllocationStatus, err := fetcher.VMAllocationStatus(ctx) + machineVMAllocationStatus, err := azurefetcher.VMAllocationStatus(ctx) if err != nil { p.internalServerError(w, err) return diff --git a/pkg/portal/cluster/fetcher.go b/pkg/portal/cluster/fetcher.go index 7a5cc76e5..8a55bbac1 100644 --- a/pkg/portal/cluster/fetcher.go +++ b/pkg/portal/cluster/fetcher.go @@ -34,12 +34,15 @@ type VirtualMachinesClientFactory interface { // information about clusters. It returns frontend-suitable data structures. type FetchClient interface { Nodes(context.Context) (*NodeListInformation, error) - VMAllocationStatus(context.Context) (map[string]string, error) ClusterOperators(context.Context) (*ClusterOperatorsInformation, error) Machines(context.Context) (*MachineListInformation, error) MachineSets(context.Context) (*MachineSetListInformation, error) } +type AzureFetchClient interface { + VMAllocationStatus(context.Context) (map[string]string, error) +} + // client is an implementation of FetchClient. It currently contains a "fetcher" // which is responsible for fetching information from the k8s clusters. The // mechanism of fetching the data from the cluster and returning it to the @@ -56,19 +59,28 @@ type client struct { // contains Kubernetes clients and returns the frontend-suitable data // structures. The concrete implementation of FetchClient wraps this. type realFetcher struct { - log *logrus.Entry - configCli configclient.Interface - kubernetesCli kubernetes.Interface - machineClient machineclient.Interface - azureSideFetcher azureSideFetcher - resourceClientFactory ResourceClientFactory - virtualMachinesClientFactory VirtualMachinesClientFactory + log *logrus.Entry + configCli configclient.Interface + kubernetesCli kubernetes.Interface + machineClient machineclient.Interface } +// azureClient is the same implementation as client's, the only difference is that it will be used to fetch something from azure regarding a cluster. +type azureClient struct { + log *logrus.Entry + fetcher *azureSideFetcher +} + +// azureSideFetcher is responsible for fetching information about azure resources of a k8s cluster. It +// contains azure related authentication/authorization data and returns the frontend-suitable data +// structures. The concrete implementation of AzureFetchClient wraps this. type azureSideFetcher struct { - resourceGroupName string - subscriptionDoc *api.SubscriptionDocument - env env.Interface + log *logrus.Entry + resourceGroupName string + subscriptionDoc *api.SubscriptionDocument + env env.Interface + resourceClientFactory ResourceClientFactory + virtualMachinesClientFactory VirtualMachinesClientFactory } type clientFactory struct{} @@ -81,15 +93,18 @@ func (cf clientFactory) NewVirtualMachinesClient(environment *azureclient.AROEnv return compute.NewVirtualMachinesClient(environment, subscriptionID, authorizer) } -func newAzureSideFetcher(resourceGroupName string, subscriptionDoc *api.SubscriptionDocument, env env.Interface) azureSideFetcher { +func newAzureSideFetcher(log *logrus.Entry, resourceGroupName string, subscriptionDoc *api.SubscriptionDocument, env env.Interface, resourceClientFactory ResourceClientFactory, virtualMachinesClientFactory VirtualMachinesClientFactory) azureSideFetcher { return azureSideFetcher{ - resourceGroupName: resourceGroupName, - subscriptionDoc: subscriptionDoc, - env: env, + log: log, + resourceGroupName: resourceGroupName, + subscriptionDoc: subscriptionDoc, + env: env, + resourceClientFactory: resourceClientFactory, + virtualMachinesClientFactory: virtualMachinesClientFactory, } } -func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument, azureSideFetcher azureSideFetcher, resourceClientFactory ResourceClientFactory, virtualMachinesClientFactory VirtualMachinesClientFactory) (*realFetcher, error) { +func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftClusterDocument) (*realFetcher, error) { restConfig, err := restconfig.RestConfig(dialer, doc.OpenShiftCluster) if err != nil { log.Error(err) @@ -114,21 +129,15 @@ func newRealFetcher(log *logrus.Entry, dialer proxy.Dialer, doc *api.OpenShiftCl } return &realFetcher{ - log: log, - configCli: configCli, - kubernetesCli: kubernetesCli, - machineClient: machineClient, - azureSideFetcher: azureSideFetcher, - resourceClientFactory: resourceClientFactory, - virtualMachinesClientFactory: virtualMachinesClientFactory, + log: log, + configCli: configCli, + kubernetesCli: kubernetesCli, + machineClient: machineClient, }, nil } -func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, env env.Interface) (FetchClient, error) { - resourceGroupName := stringutils.LastTokenByte(cluster.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/') - azureSideFetcher := newAzureSideFetcher(resourceGroupName, subscriptionDoc, env) - cf := clientFactory{} - fetcher, err := newRealFetcher(log, dialer, cluster, azureSideFetcher, cf, cf) +func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShiftClusterDocument) (FetchClient, error) { + fetcher, err := newRealFetcher(log, dialer, cluster) if err != nil { return nil, err } @@ -139,3 +148,13 @@ func NewFetchClient(log *logrus.Entry, dialer proxy.Dialer, cluster *api.OpenShi fetcher: fetcher, }, nil } + +func NewAzureFetchClient(log *logrus.Entry, cluster *api.OpenShiftClusterDocument, subscriptionDoc *api.SubscriptionDocument, env env.Interface) AzureFetchClient { + resourceGroupName := stringutils.LastTokenByte(cluster.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/') + cf := clientFactory{} + azureSideFetcher := newAzureSideFetcher(log, resourceGroupName, subscriptionDoc, env, cf, cf) + return &azureClient{ + log: log, + fetcher: &azureSideFetcher, + } +} diff --git a/pkg/portal/cluster/machines.go b/pkg/portal/cluster/machines.go index 6b236361f..07a361188 100644 --- a/pkg/portal/cluster/machines.go +++ b/pkg/portal/cluster/machines.go @@ -65,25 +65,26 @@ func (c *client) Machines(ctx context.Context) (*MachineListInformation, error) return c.fetcher.Machines(ctx) } -func (c *client) VMAllocationStatus(ctx context.Context) (map[string]string, error) { +func (c *azureClient) VMAllocationStatus(ctx context.Context) (map[string]string, error) { return c.fetcher.vmAllocationStatus(ctx) } -func (f *realFetcher) vmAllocationStatus(ctx context.Context) (map[string]string, error) { - env := f.azureSideFetcher.env - subscriptionDoc := f.azureSideFetcher.subscriptionDoc - clusterRGName := f.azureSideFetcher.resourceGroupName - fpAuth, err := env.FPAuthorizer(subscriptionDoc.Subscription.Properties.TenantID, env.Environment().ResourceManagerEndpoint) +func (f *azureSideFetcher) vmAllocationStatus(ctx context.Context) (map[string]string, error) { + env := f.env + subscriptionDoc := f.subscriptionDoc + clusterRGName := f.resourceGroupName + aroEnvironment := env.Environment() + fpAuth, err := env.FPAuthorizer(subscriptionDoc.Subscription.Properties.TenantID, aroEnvironment.ResourceManagerEndpoint) if err != nil { return nil, err } // Getting Virtual Machine resources through the Cluster's Resource Group - computeResources, err := f.resourceClientFactory.NewResourcesClient(env.Environment(), subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil) + computeResources, err := f.resourceClientFactory.NewResourcesClient(aroEnvironment, subscriptionDoc.ID, fpAuth).ListByResourceGroup(ctx, clusterRGName, "resourceType eq 'Microsoft.Compute/virtualMachines'", "", nil) if err != nil { return nil, err } vmAllocationStatus := make(map[string]string) - virtualMachineClient := f.virtualMachinesClientFactory.NewVirtualMachinesClient(env.Environment(), subscriptionDoc.ID, fpAuth) + virtualMachineClient := f.virtualMachinesClientFactory.NewVirtualMachinesClient(aroEnvironment, subscriptionDoc.ID, fpAuth) for _, res := range computeResources { putAllocationStatusToMap(ctx, clusterRGName, vmAllocationStatus, res, virtualMachineClient, f.log) } @@ -107,7 +108,6 @@ func putAllocationStatusToMap(ctx context.Context, clusterRGName string, vmAlloc return } } - vmAllocationStatus[vmName] = "" } diff --git a/pkg/portal/cluster/machines_test.go b/pkg/portal/cluster/machines_test.go index 414af41f6..a2b848290 100644 --- a/pkg/portal/cluster/machines_test.go +++ b/pkg/portal/cluster/machines_test.go @@ -132,88 +132,36 @@ func (mcf MockClientFactory) NewVirtualMachinesClient(environment *azureclient.A func TestVMAllocationStatus(t *testing.T) { ctx := context.Background() controller := gomock.NewController(t) - mockResourcesClient := mock_features.NewMockResourcesClient(controller) - mockVirtualMachinesClient := mock_compute.NewMockVirtualMachinesClient(controller) type test struct { name string - mocks func(*test, *mock_env.MockInterface, *mock_refreshable.MockAuthorizer, *mock_features.MockResourcesClient, *mock_compute.MockVirtualMachinesClient) + mocks map[string]int wantErr string } for _, tt := range []*test{ { name: "Successfully fetching VMs allocation status. Calling all the required methods.", - mocks: func(tt *test, - env *mock_env.MockInterface, - authorizer *mock_refreshable.MockAuthorizer, - mockResourcesClient *mock_features.MockResourcesClient, - mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) { - env.EXPECT().Environment().Return(&azureclient.AROEnvironment{ - Environment: azure.Environment{ - ResourceManagerEndpoint: "temp", - }, - }).AnyTimes() - env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(authorizer, nil) - - mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - Return([]mgmtfeatures.GenericResourceExpanded{ - { - Kind: func(v string) *string { return &v }("something"), - Type: func(v string) *string { return &v }("Microsoft.Compute/virtualMachines"), - Name: func(v string) *string { return &v }("master-x"), - }, - }, nil) - - mockVirtualMachinesClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mgmtcompute.VirtualMachine{ - Name: func(v string) *string { return &v }("master-x"), - VirtualMachineProperties: &mgmtcompute.VirtualMachineProperties{ - InstanceView: &mgmtcompute.VirtualMachineInstanceView{ - Statuses: &[]mgmtcompute.InstanceViewStatus{ - { - Code: func() *string { - s := new(string) - *s = "PowerState/running" - return s - }(), - }, - }, - }, - }, - }, nil) + mocks: map[string]int{ + "env.Environment": 1, + "env.FPAuthorizer": 1, + "resourceClient.ListByResourceGroup": 1, + "virtualMachinesClient.Get": 1, }, wantErr: "", }, { name: "No VM resource found", - mocks: func(tt *test, - env *mock_env.MockInterface, - authorizer *mock_refreshable.MockAuthorizer, - mockResourcesClient *mock_features.MockResourcesClient, - mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) { - env.EXPECT().Environment().Return(&azureclient.AROEnvironment{ - Environment: azure.Environment{ - ResourceManagerEndpoint: "temp", - }, - }).AnyTimes() - env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(authorizer, nil) - - mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - Return([]mgmtfeatures.GenericResourceExpanded{}, nil) + mocks: map[string]int{ + "env.Environment": 1, + "env.FPAuthorizer": 1, + "resourceClient.ListByResourceGroup": 1, }, wantErr: "", }, { name: "Empty FP Authorizer", - mocks: func(tt *test, - env *mock_env.MockInterface, - authorizer *mock_refreshable.MockAuthorizer, - mockResourcesClient *mock_features.MockResourcesClient, - mockVirtualMachinesClient *mock_compute.MockVirtualMachinesClient) { - env.EXPECT().Environment().Return(&azureclient.AROEnvironment{ - Environment: azure.Environment{ - ResourceManagerEndpoint: "temp", - }, - }).AnyTimes() - env.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(nil, errors.New("Empty Athorizer")) + mocks: map[string]int{ + "env.Environment": 1, + "env.FPAuthorizer": 1, }, wantErr: "Empty Athorizer", }, @@ -221,6 +169,8 @@ func TestVMAllocationStatus(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mockEnv := mock_env.NewMockInterface(controller) mockRefreshable := mock_refreshable.NewMockAuthorizer(controller) + mockResourcesClient := mock_features.NewMockResourcesClient(controller) + mockVirtualMachinesClient := mock_compute.NewMockVirtualMachinesClient(controller) subscriptionDoc := &api.SubscriptionDocument{ ID: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", ResourceID: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx", @@ -240,23 +190,77 @@ func TestVMAllocationStatus(t *testing.T) { } if tt.mocks != nil { - tt.mocks(tt, mockEnv, mockRefreshable, mockResourcesClient, mockVirtualMachinesClient) + if tt.mocks["env.Environment"] > 0 { + switch tt.name { + case "Successfully fetching VMs allocation status. Calling all the required methods.", "No VM resource found", "Empty FP Authorizer": + mockEnv.EXPECT().Environment().Return(&azureclient.AROEnvironment{ + Environment: azure.Environment{ + ResourceManagerEndpoint: "temp", + }, + }) + } + } + + if tt.mocks["env.FPAuthorizer"] > 0 { + switch tt.name { + case "Successfully fetching VMs allocation status. Calling all the required methods.", "No VM resource found": + mockEnv.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(mockRefreshable, nil) + + case "Empty FP Authorizer": + mockEnv.EXPECT().FPAuthorizer(gomock.Any(), gomock.Any()).Return(nil, errors.New("Empty Athorizer")) + } + } + + if tt.mocks["resourceClient.ListByResourceGroup"] > 0 { + switch tt.name { + case "Successfully fetching VMs allocation status. Calling all the required methods.": + mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return([]mgmtfeatures.GenericResourceExpanded{ + { + Kind: func(v string) *string { return &v }("something"), + Type: func(v string) *string { return &v }("Microsoft.Compute/virtualMachines"), + Name: func(v string) *string { return &v }("master-x"), + }, + }, nil) + case "No VM resource found": + mockResourcesClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return([]mgmtfeatures.GenericResourceExpanded{}, nil) + } + } + if tt.mocks["virtualMachinesClient.Get"] > 0 { + switch tt.name { + case "Successfully fetching VMs allocation status. Calling all the required methods.": + mockVirtualMachinesClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(mgmtcompute.VirtualMachine{ + Name: func(v string) *string { return &v }("master-x"), + VirtualMachineProperties: &mgmtcompute.VirtualMachineProperties{ + InstanceView: &mgmtcompute.VirtualMachineInstanceView{ + Statuses: &[]mgmtcompute.InstanceViewStatus{ + { + Code: func() *string { + s := new(string) + *s = "PowerState/running" + return s + }(), + }, + }, + }, + }, + }, nil) + } + } } _, log := testlog.New() - azureSideFetcher := azureSideFetcher{ - resourceGroupName: "someResourceGroup", - env: mockEnv, - subscriptionDoc: subscriptionDoc, - } mcf := newMockClientFactory(mockResourcesClient, mockVirtualMachinesClient) - realFetcher := &realFetcher{ + azureSideFetcher := &azureSideFetcher{ log: log, - azureSideFetcher: azureSideFetcher, + resourceGroupName: "someResourceGroup", + subscriptionDoc: subscriptionDoc, + env: mockEnv, resourceClientFactory: mcf, virtualMachinesClientFactory: mcf, } - client := &client{fetcher: realFetcher, log: log} - _, err := client.VMAllocationStatus(ctx) + azureClient := &azureClient{fetcher: azureSideFetcher, log: log} + _, err := azureClient.VMAllocationStatus(ctx) if err != nil && err.Error() != tt.wantErr || err == nil && tt.wantErr != "" { t.Error("Expected", tt.wantErr, "Got", err) } diff --git a/pkg/portal/portal.go b/pkg/portal/portal.go index 4f91764e0..c2ce56e95 100644 --- a/pkg/portal/portal.go +++ b/pkg/portal/portal.go @@ -47,16 +47,13 @@ type Runnable interface { } type portal struct { - env env.Interface - audit *logrus.Entry - log *logrus.Entry - baseAccessLog *logrus.Entry - l net.Listener - sshl net.Listener - verifier oidc.Verifier - baseRouter *mux.Router - authenticatedRouter *mux.Router - publicRouter *mux.Router + env env.Interface + audit *logrus.Entry + log *logrus.Entry + baseAccessLog *logrus.Entry + l net.Listener + sshl net.Listener + verifier oidc.Verifier hostname string servingKey *rsa.PrivateKey @@ -400,13 +397,37 @@ func (p *portal) makeFetcher(ctx context.Context, r *http.Request) (cluster.Fetc } else { dialer = p.dialer } + return cluster.NewFetchClient(p.log, dialer, doc) +} + +// makeAzureFetcher creates a cluster.AzureFetchClient suitable for use by the Portal REST API to fetch anything directly from Azure like VM Details etc. +func (p *portal) makeAzureFetcher(ctx context.Context, r *http.Request) (cluster.AzureFetchClient, error) { + apiVars := mux.Vars(r) + + subscription := apiVars["subscription"] + resourceGroup := apiVars["resourceGroup"] + clusterName := apiVars["clusterName"] + + resourceID := + strings.ToLower( + fmt.Sprintf( + "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.RedHatOpenShift/openShiftClusters/%s", + subscription, resourceGroup, clusterName)) + if !validate.RxClusterID.MatchString(resourceID) { + return nil, fmt.Errorf("invalid resource ID") + } + + doc, err := p.dbOpenShiftClusters.Get(ctx, resourceID) + if err != nil { + return nil, err + } subscriptionDoc, err := p.getSubscriptionDocument(ctx, doc.Key) if err != nil { return nil, err } - return cluster.NewFetchClient(p.log, dialer, doc, subscriptionDoc, p.env) + return cluster.NewAzureFetchClient(p.log, doc, subscriptionDoc, p.env), nil } func (p *portal) serve(path string) func(w http.ResponseWriter, r *http.Request) {