Unverified Commit d3871bcd authored by Vihang Mehta's avatar Vihang Mehta Committed by Copybara
Browse files

Remove and reserve vizier configs


Summary:
This prevents any clients from fataling, but also ensures that clients
don't need to send config values.

Always respond with passthrough true from cloudapi so that older clients continue
to work as expected.

Test Plan: Deploy to staging and test.

Reviewers: michelle, zasgar

Reviewed By: michelle
Signed-off-by: default avatarVihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D11859

GitOrigin-RevId: 3f90a596126089b0663162cba9177aca5e97da6a
No related merge requests found
Showing with 742 additions and 1252 deletions
+742 -1252
This diff is collapsed.
......@@ -272,13 +272,11 @@ service VizierClusterInfo {
message VizierConfig {
bool passthrough_enabled = 1;
bool auto_update_enabled = 2;
reserved 2;
}
message VizierConfigUpdate {
// Deprecated: We no longer let users disable passthrough. This will be removed in a future release.
google.protobuf.BoolValue passthrough_enabled = 1;
reserved 2;
reserved 1, 2;
}
message GetClusterInfoRequest {
......
......@@ -194,8 +194,7 @@ func (v *VizierClusterInfo) getClusterInfoForViziers(ctx context.Context, ids []
StatusMessage: vzInfo.StatusMessage,
LastHeartbeatNs: vzInfo.LastHeartbeatNs,
Config: &cloudpb.VizierConfig{
PassthroughEnabled: vzInfo.Config.PassthroughEnabled,
AutoUpdateEnabled: vzInfo.Config.AutoUpdateEnabled,
PassthroughEnabled: true,
},
ClusterUID: vzInfo.ClusterUID,
ClusterName: vzInfo.ClusterName,
......@@ -241,21 +240,6 @@ func (v *VizierClusterInfo) GetClusterConnectionInfo(ctx context.Context, reques
// UpdateClusterVizierConfig supports updates of VizierConfig for a cluster
func (v *VizierClusterInfo) UpdateClusterVizierConfig(ctx context.Context, req *cloudpb.UpdateClusterVizierConfigRequest) (*cloudpb.UpdateClusterVizierConfigResponse, error) {
ctx, err := contextWithAuthToken(ctx)
if err != nil {
return nil, err
}
_, err = v.VzMgr.UpdateVizierConfig(ctx, &cvmsgspb.UpdateVizierConfigRequest{
VizierID: req.ID,
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: req.ConfigUpdate.PassthroughEnabled,
},
})
if err != nil {
return nil, err
}
return &cloudpb.UpdateClusterVizierConfigResponse{}, nil
}
......
......@@ -120,14 +120,11 @@ func TestVizierClusterInfo_GetClusterInfo(t *testing.T) {
Status: cvmsgspb.VZ_ST_HEALTHY,
StatusMessage: "Everything is running",
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: false,
AutoUpdateEnabled: true,
},
VizierVersion: "1.2.3",
ClusterUID: "a UID",
ClusterName: "gke_pl-dev-infra_us-west1-a_dev-cluster-zasgar-3",
ClusterVersion: "5.6.7",
Config: &cvmsgspb.VizierConfig{},
VizierVersion: "1.2.3",
ClusterUID: "a UID",
ClusterName: "gke_pl-dev-infra_us-west1-a_dev-cluster-zasgar-3",
ClusterVersion: "5.6.7",
ControlPlanePodStatuses: map[string]*cvmsgspb.PodStatus{
"vizier-proxy": {
Name: "vizier-proxy",
......@@ -205,8 +202,6 @@ func TestVizierClusterInfo_GetClusterInfo(t *testing.T) {
assert.Equal(t, cluster.ID, clusterID)
assert.Equal(t, cluster.Status, cloudpb.CS_HEALTHY)
assert.Equal(t, cluster.LastHeartbeatNs, int64(1305646598000000000))
assert.Equal(t, cluster.Config.PassthroughEnabled, false)
assert.Equal(t, cluster.Config.AutoUpdateEnabled, true)
assert.Equal(t, "1.2.3", cluster.VizierVersion)
assert.Equal(t, "a UID", cluster.ClusterUID)
assert.Equal(t, "gke_pl-dev-infra_us-west1-a_dev-cluster-zasgar-3", cluster.ClusterName)
......@@ -257,13 +252,10 @@ func TestVizierClusterInfo_GetClusterInfoDuplicates(t *testing.T) {
VizierIDs: []*uuidpb.UUID{clusterID, clusterID2},
}).Return(&vzmgrpb.GetVizierInfosResponse{
VizierInfos: []*cvmsgspb.VizierInfo{{
VizierID: clusterID,
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: false,
AutoUpdateEnabled: true,
},
VizierID: clusterID,
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{},
VizierVersion: "1.2.3",
ClusterUID: "a UID",
ClusterName: "gke_pl-dev-infra_us-west1-a_dev-cluster-zasgar",
......@@ -272,13 +264,10 @@ func TestVizierClusterInfo_GetClusterInfoDuplicates(t *testing.T) {
NumInstrumentedNodes: 3,
},
{
VizierID: clusterID,
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: false,
AutoUpdateEnabled: true,
},
VizierID: clusterID,
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{},
VizierVersion: "1.2.3",
ClusterUID: "a UID2",
ClusterName: "gke_pl-pixies_us-west1-a_dev-cluster-zasgar",
......@@ -337,14 +326,11 @@ func TestVizierClusterInfo_GetClusterInfoWithID(t *testing.T) {
VizierID: clusterID,
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: int64(1305646598000000000),
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: false,
AutoUpdateEnabled: true,
},
VizierVersion: "1.2.3",
ClusterUID: "a UID",
ClusterName: "some cluster",
ClusterVersion: "5.6.7",
Config: &cvmsgspb.VizierConfig{},
VizierVersion: "1.2.3",
ClusterUID: "a UID",
ClusterName: "some cluster",
ClusterVersion: "5.6.7",
},
},
}, nil)
......@@ -363,8 +349,6 @@ func TestVizierClusterInfo_GetClusterInfoWithID(t *testing.T) {
assert.Equal(t, cluster.ID, clusterID)
assert.Equal(t, cluster.Status, cloudpb.CS_HEALTHY)
assert.Equal(t, cluster.LastHeartbeatNs, int64(1305646598000000000))
assert.Equal(t, cluster.Config.PassthroughEnabled, false)
assert.Equal(t, cluster.Config.AutoUpdateEnabled, true)
})
}
}
......@@ -396,24 +380,13 @@ func TestVizierClusterInfo_UpdateClusterVizierConfig(t *testing.T) {
defer cleanup()
ctx := test.ctx
updateReq := &cvmsgspb.UpdateVizierConfigRequest{
VizierID: clusterID,
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: true},
},
}
mockClients.MockVzMgr.EXPECT().UpdateVizierConfig(gomock.Any(), updateReq).Return(&cvmsgspb.UpdateVizierConfigResponse{}, nil)
vzClusterInfoServer := &controllers.VizierClusterInfo{
VzMgr: mockClients.MockVzMgr,
}
resp, err := vzClusterInfoServer.UpdateClusterVizierConfig(ctx, &cloudpb.UpdateClusterVizierConfigRequest{
ID: clusterID,
ConfigUpdate: &cloudpb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: true},
},
ID: clusterID,
ConfigUpdate: &cloudpb.VizierConfigUpdate{},
})
require.NoError(t, err)
......
......@@ -48,8 +48,6 @@ var (
ErrCredentialGenerate = status.Error(codes.Internal, "failed to generate creds for cluster")
// ErrPermissionDenied occurs when permission is denied to the cluster.
ErrPermissionDenied = status.Error(codes.PermissionDenied, "permission denied for access to cluster")
// ErrPassthroughDisabled occurs when a passthrough request is made to a Vizier that is sent to Direct mode.
ErrPassthroughDisabled = status.Error(codes.Unavailable, "cluster is not in passthrough mode")
)
// requestProxyer manages a single proxy request.
......@@ -149,9 +147,6 @@ func (p requestProxyer) validateRequestAndFetchCreds(ctx context.Context, debugM
}
}
if resp.Config != nil && !resp.Config.PassthroughEnabled {
return ErrPassthroughDisabled
}
return nil
})
......
......@@ -731,9 +731,7 @@ func (v *fakeVzMgr) GetVizierInfo(ctx context.Context, in *uuidpb.UUID, opts ...
VizierID: utils.ProtoFromUUIDStrOrNil("00000000-1111-2222-2222-333333333333"),
Status: cvmsgspb.VZ_ST_HEALTHY,
LastHeartbeatNs: 0,
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: true,
},
Config: &cvmsgspb.VizierConfig{},
},
nil,
},
......@@ -742,9 +740,7 @@ func (v *fakeVzMgr) GetVizierInfo(ctx context.Context, in *uuidpb.UUID, opts ...
VizierID: utils.ProtoFromUUIDStrOrNil("10000000-1111-2222-2222-333333333333"),
Status: cvmsgspb.VZ_ST_DISCONNECTED,
LastHeartbeatNs: 0,
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: true,
},
Config: &cvmsgspb.VizierConfig{},
},
nil,
},
......@@ -753,9 +749,7 @@ func (v *fakeVzMgr) GetVizierInfo(ctx context.Context, in *uuidpb.UUID, opts ...
VizierID: utils.ProtoFromUUIDStrOrNil("10000000-1111-2222-2222-333333333333"),
Status: cvmsgspb.VZ_ST_UNHEALTHY,
LastHeartbeatNs: 0,
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: true,
},
Config: &cvmsgspb.VizierConfig{},
},
nil,
},
......@@ -764,9 +758,7 @@ func (v *fakeVzMgr) GetVizierInfo(ctx context.Context, in *uuidpb.UUID, opts ...
VizierID: utils.ProtoFromUUIDStrOrNil("30000000-1111-2222-2222-333333333333"),
Status: cvmsgspb.VZ_ST_UNHEALTHY,
LastHeartbeatNs: 0,
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: false,
},
Config: &cvmsgspb.VizierConfig{},
},
nil,
},
......
......@@ -303,13 +303,10 @@ func vizierInfoToProto(vzInfo VizierInfo) *cvmsgspb.VizierInfo {
}
return &cvmsgspb.VizierInfo{
VizierID: utils.ProtoFromUUID(vzInfo.ID),
Status: vzInfo.Status.ToProto(),
LastHeartbeatNs: lastHearbeat,
Config: &cvmsgspb.VizierConfig{
PassthroughEnabled: vzInfo.PassthroughEnabled,
AutoUpdateEnabled: vzInfo.AutoUpdateEnabled,
},
VizierID: utils.ProtoFromUUID(vzInfo.ID),
Status: vzInfo.Status.ToProto(),
LastHeartbeatNs: lastHearbeat,
Config: &cvmsgspb.VizierConfig{},
ClusterUID: clusterUID,
ClusterName: clusterName,
ClusterVersion: clusterVersion,
......@@ -426,74 +423,12 @@ func (s *Server) GetVizierInfo(ctx context.Context, req *uuidpb.UUID) (*cvmsgspb
return nil, status.Error(codes.NotFound, "vizier not found")
}
// getVizierConfig returns the current Vizier config.
// WARNING: This doesn't check validateOrgOwnsCluster since
// the certmgr usecase cannot get a valid authcontext from the passed in
// context.
func (s *Server) getVizierConfig(ctx context.Context, vizierIDPb *uuidpb.UUID) (*cvmsgspb.VizierConfig, error) {
vizierID := utils.UUIDFromProtoOrNil(vizierIDPb)
query := `
SELECT passthrough_enabled, auto_update_enabled
FROM vizier_cluster_info
WHERE vizier_cluster_id = $1`
var val struct {
PassthroughEnabled bool `db:"passthrough_enabled"`
AutoUpdateEnabled bool `db:"auto_update_enabled"`
}
err := s.db.Get(&val, query, vizierID)
if err != nil {
if err == sql.ErrNoRows {
return nil, status.Error(codes.NotFound, "no such cluster")
}
return nil, err
}
return &cvmsgspb.VizierConfig{
PassthroughEnabled: val.PassthroughEnabled,
AutoUpdateEnabled: val.AutoUpdateEnabled,
}, nil
}
// UpdateVizierConfig supports updating of the Vizier config.
func (s *Server) UpdateVizierConfig(ctx context.Context, req *cvmsgspb.UpdateVizierConfigRequest) (*cvmsgspb.UpdateVizierConfigResponse, error) {
if err := s.validateOrgOwnsCluster(ctx, req.VizierID); err != nil {
return nil, err
}
vizierID := utils.UUIDFromProtoOrNil(req.VizierID)
if req.ConfigUpdate == nil {
return &cvmsgspb.UpdateVizierConfigResponse{}, nil
}
currentConfig, err := s.getVizierConfig(ctx, req.VizierID)
if err != nil {
return nil, err
}
ptEnabled := currentConfig.PassthroughEnabled
if req.ConfigUpdate.PassthroughEnabled != nil {
if !req.ConfigUpdate.PassthroughEnabled.Value {
return nil, status.Error(codes.InvalidArgument, "Deprecated. Disabling passthrough is no longer supported and is being phased out.")
}
ptEnabled = req.ConfigUpdate.PassthroughEnabled.Value
}
query := `
UPDATE vizier_cluster_info
SET passthrough_enabled = $1
WHERE vizier_cluster_id = $2`
res, err := s.db.Exec(query, ptEnabled, vizierID)
if err != nil {
return nil, err
}
if count, _ := res.RowsAffected(); count == 0 {
return nil, status.Error(codes.NotFound, "no such cluster")
}
return &cvmsgspb.UpdateVizierConfigResponse{}, nil
}
......
......@@ -242,8 +242,6 @@ func TestServer_GetVizierInfo(t *testing.T) {
assert.Equal(t, resp.VizierID, utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001"))
assert.Equal(t, resp.Status, cvmsgspb.VZ_ST_HEALTHY)
assert.Greater(t, resp.LastHeartbeatNs, int64(0))
assert.Equal(t, resp.Config.PassthroughEnabled, false)
assert.Equal(t, resp.Config.AutoUpdateEnabled, true)
assert.Equal(t, "vzVers", resp.VizierVersion)
assert.Equal(t, "cVers", resp.ClusterVersion)
assert.Equal(t, "healthy_cluster", resp.ClusterName)
......@@ -294,106 +292,6 @@ func TestServer_GetVizierInfos(t *testing.T) {
assert.Equal(t, "k8sID", resp.VizierInfos[3].ClusterUID)
}
func TestServer_UpdateVizierConfig(t *testing.T) {
mustLoadTestData(db)
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := controllers.New(db, "test", nil, nil)
vzIDpb := utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001")
resp, err := s.UpdateVizierConfig(CreateTestContext(), &cvmsgspb.UpdateVizierConfigRequest{
VizierID: vzIDpb,
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: true},
},
})
require.NoError(t, err)
require.NotNil(t, resp)
// Check that the value was actually updated.
infoResp, err := s.GetVizierInfo(CreateTestContext(), vzIDpb)
require.NoError(t, err)
require.NotNil(t, infoResp)
assert.Equal(t, infoResp.Config.PassthroughEnabled, true)
}
func TestServer_UpdateVizierConfig_PassthroughDisable(t *testing.T) {
mustLoadTestData(db)
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := controllers.New(db, "test", nil, nil)
vzIDpb := utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001")
_, err := s.UpdateVizierConfig(CreateTestContext(), &cvmsgspb.UpdateVizierConfigRequest{
VizierID: vzIDpb,
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: false},
},
})
require.NotNil(t, err)
assert.Equal(t, status.Code(err), codes.InvalidArgument)
}
func TestServer_UpdateVizierConfig_PassthroughEnable(t *testing.T) {
mustLoadTestData(db)
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := controllers.New(db, "test", nil, nil)
vzIDpb := utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001")
_, err := s.UpdateVizierConfig(CreateTestContext(), &cvmsgspb.UpdateVizierConfigRequest{
VizierID: vzIDpb,
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: true},
},
})
require.NoError(t, err)
}
func TestServer_UpdateVizierConfig_WrongOrg(t *testing.T) {
mustLoadTestData(db)
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := controllers.New(db, "test", nil, nil)
resp, err := s.UpdateVizierConfig(CreateTestContext(), &cvmsgspb.UpdateVizierConfigRequest{
VizierID: utils.ProtoFromUUIDStrOrNil("223e4567-e89b-12d3-a456-426655440003"),
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{
PassthroughEnabled: &types.BoolValue{Value: true},
},
})
require.Nil(t, resp)
require.NotNil(t, err)
assert.Equal(t, status.Code(err), codes.NotFound)
}
func TestServer_UpdateVizierConfig_NoUpdates(t *testing.T) {
mustLoadTestData(db)
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := controllers.New(db, "test", nil, nil)
resp, err := s.UpdateVizierConfig(CreateTestContext(), &cvmsgspb.UpdateVizierConfigRequest{
VizierID: utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001"),
ConfigUpdate: &cvmsgspb.VizierConfigUpdate{},
})
require.NoError(t, err)
require.NotNil(t, resp)
// Check that the value was not updated.
infoResp, err := s.GetVizierInfo(CreateTestContext(), utils.ProtoFromUUIDStrOrNil("123e4567-e89b-12d3-a456-426655440001"))
require.NoError(t, err)
require.NotNil(t, infoResp)
assert.Equal(t, infoResp.Config.PassthroughEnabled, false)
}
func TestServer_GetVizierConnectionInfo(t *testing.T) {
mustLoadTestData(db)
viper.Set("domain_name", "withpixie.ai")
......
This diff is collapsed.
......@@ -35,7 +35,6 @@ option go_package = "cvmsgspb";
import "github.com/gogo/protobuf/gogoproto/gogo.proto";
import "google/protobuf/any.proto";
import "google/protobuf/timestamp.proto";
import "google/protobuf/wrappers.proto";
import "src/api/proto/uuidpb/uuid.proto";
import "src/api/proto/vizierpb/vizierapi.proto";
import "src/shared/k8s/metadatapb/metadata.proto";
......@@ -188,14 +187,11 @@ message VizierHeartbeatAck {
}
message VizierConfig {
bool passthrough_enabled = 1;
bool auto_update_enabled = 2;
reserved 1, 2;
}
message VizierConfigUpdate {
// Deprecated: We no longer let users disable passthrough. This will be removed in a future release.
google.protobuf.BoolValue passthrough_enabled = 1;
reserved 2;
reserved 1, 2;
}
message VizierInfo {
......
This diff is collapsed.
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment