Bug 2255499: [release-4.15] Onboard clients only if they are on same version as provider - 2

services/provider/client/client.go

@@ -6,7 +6,7 @@ import (
 	"fmt"
 	"time"
 
-	cs "github.com/red-hat-storage/ocs-operator/v4/services/provider/clientstatus"
+	ifaces "github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces"
 	pb "github.com/red-hat-storage/ocs-operator/v4/services/provider/pb"
 
 	"google.golang.org/grpc"
@@ -53,17 +53,18 @@ func (cc *OCSProviderClient) Close() {
 	cc.Client = nil
 }
 
+func NewOnboardConsumerRequest() ifaces.StorageClientOnboarding {
+	return &pb.OnboardConsumerRequest{}
+}
+
 // OnboardConsumer to validate the consumer and create StorageConsumer
 // resource on the StorageProvider cluster
-func (cc *OCSProviderClient) OnboardConsumer(ctx context.Context, ticket, name string) (*pb.OnboardConsumerResponse, error) {
+func (cc *OCSProviderClient) OnboardConsumer(ctx context.Context, onboard ifaces.StorageClientOnboarding) (*pb.OnboardConsumerResponse, error) {
 	if cc.Client == nil || cc.clientConn == nil {
 		return nil, fmt.Errorf("provider client is closed")
 	}
 
-	req := &pb.OnboardConsumerRequest{
-		OnboardingTicket: ticket,
-		ConsumerName:     name,
-	}
+	req := onboard.(*pb.OnboardConsumerRequest)
 
 	apiCtx, cancel := context.WithTimeout(ctx, cc.timeout)
 	defer cancel()
@@ -189,11 +190,11 @@ func (cc *OCSProviderClient) GetStorageClassClaimConfig(ctx context.Context, con
 	return cc.Client.GetStorageClassClaimConfig(apiCtx, req)
 }
 
-func NewStorageClientStatus() cs.StorageClientStatus {
+func NewStorageClientStatus() ifaces.StorageClientStatus {
 	return &pb.ReportStatusRequest{}
 }
 
-func (cc *OCSProviderClient) ReportStatus(ctx context.Context, consumerUUID string, status cs.StorageClientStatus) (*pb.ReportStatusResponse, error) {
+func (cc *OCSProviderClient) ReportStatus(ctx context.Context, consumerUUID string, status ifaces.StorageClientStatus) (*pb.ReportStatusResponse, error) {
 	if cc.Client == nil || cc.clientConn == nil {
 		return nil, fmt.Errorf("Provider client is closed")
 	}

services/provider/interfaces/interfaces.go

@@ -0,0 +1,23 @@
+package interfaces
+
+type StorageClientStatus interface {
+	// TODO: it was mistake not using full name of the field and we are just
+	// doing indirection for getters, change this interface after ensuring
+	// no client is using it
+	GetPlatformVersion() string
+	GetOperatorVersion() string
+
+	SetPlatformVersion(string) StorageClientStatus
+	SetOperatorVersion(string) StorageClientStatus
+}
+
+type StorageClientOnboarding interface {
+	// getters for fields are already provided by protobuf messages
+	GetOnboardingTicket() string
+	GetConsumerName() string
+	GetClientOperatorVersion() string
+
+	SetOnboardingTicket(string) StorageClientOnboarding
+	SetConsumerName(string) StorageClientOnboarding
+	SetClientOperatorVersion(string) StorageClientOnboarding
+}

services/provider/pb/storageclient.go

@@ -0,0 +1,44 @@
+package providerpb
+
+import (
+	ifaces "github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces"
+)
+
+// ensure ReportStatusRequest satisfies StorageClientStatus interface
+var _ ifaces.StorageClientStatus = &ReportStatusRequest{}
+
+func (r *ReportStatusRequest) GetPlatformVersion() string {
+	return r.GetClientPlatformVersion()
+}
+
+func (r *ReportStatusRequest) GetOperatorVersion() string {
+	return r.GetClientOperatorVersion()
+}
+
+func (r *ReportStatusRequest) SetPlatformVersion(version string) ifaces.StorageClientStatus {
+	r.ClientPlatformVersion = version
+	return r
+}
+
+func (r *ReportStatusRequest) SetOperatorVersion(version string) ifaces.StorageClientStatus {
+	r.ClientOperatorVersion = version
+	return r
+}
+
+// ensure OnboardConsumerRequest satisfies StorageClientOnboarding interface
+var _ ifaces.StorageClientOnboarding = &OnboardConsumerRequest{}
+
+func (o *OnboardConsumerRequest) SetOnboardingTicket(ticket string) ifaces.StorageClientOnboarding {
+	o.OnboardingTicket = ticket
+	return o
+}
+
+func (o *OnboardConsumerRequest) SetConsumerName(name string) ifaces.StorageClientOnboarding {
+	o.ConsumerName = name
+	return o
+}
+
+func (o *OnboardConsumerRequest) SetClientOperatorVersion(version string) ifaces.StorageClientOnboarding {
+	o.ClientOperatorVersion = version
+	return o
+}

services/provider/proto/provider.proto

@@ -47,6 +47,8 @@ message OnboardConsumerRequest{
     string onboardingTicket = 1;
     // consumerName is the name of the consumer that is used to create the storageConsumer resource
     string consumerName = 2;
+    // clientOperatorVersion is the semver version of ocs-client-operator
+    string clientOperatorVersion = 3;
 }
 
 // OnboardConsumerResponse holds the response for OnboardConsumer API request

services/provider/server/consumer.go

@@ -7,7 +7,7 @@ import (
 	"sync"
 
 	ocsv1alpha1 "github.com/red-hat-storage/ocs-operator/api/v4/v1alpha1"
-	cs "github.com/red-hat-storage/ocs-operator/v4/services/provider/clientstatus"
+	ifaces "github.com/red-hat-storage/ocs-operator/v4/services/provider/interfaces"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -54,7 +54,9 @@ func newConsumerManager(ctx context.Context, cl client.Client, namespace string)
 }
 
 // Create creates a new storageConsumer resource, updates the consumer cache and returns the storageConsumer UID
-func (c *ocsConsumerManager) Create(ctx context.Context, name, ticket string) (string, error) {
+func (c *ocsConsumerManager) Create(ctx context.Context, onboard ifaces.StorageClientOnboarding) (string, error) {
+	ticket := onboard.GetOnboardingTicket()
+	name := onboard.GetConsumerName()
 	c.mutex.RLock()
 	if _, ok := c.nameByTicket[ticket]; ok {
 		c.mutex.RUnlock()
@@ -74,6 +76,11 @@ func (c *ocsConsumerManager) Create(ctx context.Context, name, ticket string) (s
 		Spec: ocsv1alpha1.StorageConsumerSpec{
 			Enable: false,
 		},
+		Status: ocsv1alpha1.StorageConsumerStatus{
+			Client: ocsv1alpha1.ClientStatus{
+				OperatorVersion: onboard.GetClientOperatorVersion(),
+			},
+		},
 	}
 
 	err := c.client.Create(ctx, consumerObj)
@@ -199,7 +206,7 @@ func (c *ocsConsumerManager) Get(ctx context.Context, id string) (*ocsv1alpha1.S
 	return consumerObj, nil
 }
 
-func (c *ocsConsumerManager) UpdateConsumerStatus(ctx context.Context, id string, status cs.StorageClientStatus) error {
+func (c *ocsConsumerManager) UpdateConsumerStatus(ctx context.Context, id string, status ifaces.StorageClientStatus) error {
 	consumerObj, err := c.Get(ctx, id)
 	if err != nil {
 		return err

services/provider/server/consumer_test.go

@@ -103,20 +103,41 @@ func TestCreateStorageConsumer(t *testing.T) {
 	assert.NoError(t, err)
 
 	// Create consumer should fail if consumer already exists
-	_, err = consumerManager.Create(ctx, "consumer1", "ticket1")
+	req := providerClient.NewOnboardConsumerRequest().
+		SetConsumerName("consumer1").
+		SetOnboardingTicket("ticket1")
+	_, err = consumerManager.Create(ctx, req)
 	assert.Error(t, err)
 
 	// Create consumer should fail if ticket is already used
-	_, err = consumerManager.Create(ctx, "consumer3", "ticket1")
+	req = providerClient.NewOnboardConsumerRequest().
+		SetConsumerName("consumer3").
+		SetOnboardingTicket("ticket1")
+	_, err = consumerManager.Create(ctx, req)
 	assert.Error(t, err)
 
 	// Create consumer successfully. (Can't validate the UID because fake client does not add UID)
 	assert.Equal(t, 1, len(consumerManager.nameByTicket))
-	_, err = consumerManager.Create(ctx, "consumer2", "ticket2")
+	req = providerClient.NewOnboardConsumerRequest().
+		SetConsumerName("consumer2").
+		SetOnboardingTicket("ticket2")
+	_, err = consumerManager.Create(ctx, req)
 	assert.NoError(t, err)
 	assert.Equal(t, 2, len(consumerManager.nameByTicket))
 	assert.Equal(t, "consumer1", consumerManager.nameByTicket["ticket1"])
 	assert.Equal(t, "consumer2", consumerManager.nameByTicket["ticket2"])
+
+	version := "4.15.1"
+	name := "consumer3"
+	req = providerClient.NewOnboardConsumerRequest().
+		SetConsumerName("consumer3").
+		SetOnboardingTicket("ticket3").
+		SetClientOperatorVersion(version)
+	_, err = consumerManager.Create(ctx, req)
+	assert.NoError(t, err)
+	consumerObject, err := consumerManager.GetByName(ctx, name)
+	assert.NoError(t, err)
+	assert.Equal(t, consumerObject.Status.Client.OperatorVersion, version)
 }
 
 func TestDeleteStorageConsumer(t *testing.T) {

services/provider/server/server.go

@@ -21,6 +21,7 @@ import (
 	ocsv1alpha1 "github.com/red-hat-storage/ocs-operator/api/v4/v1alpha1"
 	controllers "github.com/red-hat-storage/ocs-operator/v4/controllers/storageconsumer"
 	pb "github.com/red-hat-storage/ocs-operator/v4/services/provider/pb"
+	ocsVersion "github.com/red-hat-storage/ocs-operator/v4/version"
 	rookCephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 
 	"google.golang.org/grpc"
@@ -91,6 +92,17 @@ func NewOCSProviderServer(ctx context.Context, namespace string) (*OCSProviderSe
 // OnboardConsumer RPC call to onboard a new OCS consumer cluster.
 func (s *OCSProviderServer) OnboardConsumer(ctx context.Context, req *pb.OnboardConsumerRequest) (*pb.OnboardConsumerResponse, error) {
 
+	version, err := semver.FinalizeVersion(req.ClientOperatorVersion)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "malformed ClientOperatorVersion for client %q is provided. %v", req.ConsumerName, err)
+	}
+
+	serverVersion, _ := semver.Make(ocsVersion.Version)
+	clientVersion, _ := semver.Make(version)
+	if serverVersion.Major != clientVersion.Major || serverVersion.Minor != clientVersion.Minor {
+		return nil, status.Errorf(codes.FailedPrecondition, "both server and client %q operators major and minor versions should match for onboarding process", req.ConsumerName)
+	}
+
 	pubKey, err := s.getOnboardingValidationKey(ctx)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to get public key to validate onboarding ticket for consumer %q. %v", req.ConsumerName, err)
@@ -101,7 +113,7 @@ func (s *OCSProviderServer) OnboardConsumer(ctx context.Context, req *pb.Onboard
 		return nil, status.Errorf(codes.InvalidArgument, "onboarding ticket is not valid. %v", err)
 	}
 
-	storageConsumerUUID, err := s.consumerManager.Create(ctx, req.ConsumerName, req.OnboardingTicket)
+	storageConsumerUUID, err := s.consumerManager.Create(ctx, req)
 	if err != nil {
 		if !kerrors.IsAlreadyExists(err) && err != errTicketAlreadyExists {
 			return nil, status.Errorf(codes.Internal, "failed to create storageConsumer %q. %v", req.ConsumerName, err)