Skip to content

Commit

Permalink
Implements API scope full access check for Status OTE
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 699190596
  • Loading branch information
@kj1724 @copybara-github
kj1724 authored and copybara-github committed Nov 22, 2024
1 parent 3c644b6 commit 234eb5f
Show file tree
Hide file tree
Showing 8 changed files with 172 additions and 37 deletions.
15 changes: 15 additions & 0 deletions internal/onetime/status/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/GoogleCloudPlatform/sapagent/internal/onetime"
"github.com/GoogleCloudPlatform/sapagent/internal/onetime/supportbundle"
"github.com/GoogleCloudPlatform/sapagent/shared/commandlineexecutor"
"github.com/GoogleCloudPlatform/sapagent/shared/gce/metadataserver"
"github.com/GoogleCloudPlatform/sapagent/shared/log"
"github.com/GoogleCloudPlatform/sapagent/shared/statushelper"

Expand All @@ -43,6 +44,8 @@ const (
fetchLatestVersionError = "Error: could not fetch latest version"
)

var requiredScopes = []string{"https://www.googleapis.com/auth/cloud-platform"}

// Status stores the status subcommand parameters.
type Status struct {
ConfigFilePath string
Expand All @@ -55,6 +58,7 @@ type Status struct {
readFile configuration.ReadConfigFile
exec commandlineexecutor.Execute
exists commandlineexecutor.Exists
fetchCloudProps metadataserver.GetCloudProperties
}

// Name implements the subcommand interface for status.
Expand Down Expand Up @@ -111,6 +115,7 @@ func (s *Status) Run(ctx context.Context, opts *onetime.RunOptions) (*spb.AgentS
s.readFile = os.ReadFile
s.exec = commandlineexecutor.ExecuteCommand
s.exists = commandlineexecutor.CommandExists
s.fetchCloudProps = metadataserver.FetchCloudProperties
status, err := s.statusHandler(ctx)
if err != nil {
log.CtxLogger(ctx).Errorw("Could not get agent status", "error", err)
Expand Down Expand Up @@ -148,6 +153,16 @@ func (s *Status) agentStatus(ctx context.Context) (*spb.AgentStatus, *cpb.Config
agentStatus.AvailableVersion = fetchLatestVersionError
}

metadataCloudProps := s.fetchCloudProps()
hasRequiredScopes, err := statushelper.CheckScopes(ctx, metadataCloudProps.Scopes, requiredScopes)
agentStatus.CloudApiAccessFullScopesGranted = spb.State_FAILURE_STATE
if err != nil {
log.CtxLogger(ctx).Errorw("Could not fetch scopes", "error", err)
agentStatus.CloudApiAccessFullScopesGranted = spb.State_ERROR_STATE
} else if hasRequiredScopes {
agentStatus.CloudApiAccessFullScopesGranted = spb.State_SUCCESS_STATE
}

enabled, running, err := statushelper.CheckAgentEnabledAndRunning(ctx, agentPackageName, runtime.GOOS, s.exec)
agentStatus.SystemdServiceEnabled = spb.State_FAILURE_STATE
agentStatus.SystemdServiceRunning = spb.State_FAILURE_STATE
Expand Down
41 changes: 27 additions & 14 deletions internal/onetime/status/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
ipb "github.com/GoogleCloudPlatform/sapagent/protos/instanceinfo"
spb "github.com/GoogleCloudPlatform/sapagent/protos/status"
"github.com/GoogleCloudPlatform/sapagent/shared/commandlineexecutor"
"github.com/GoogleCloudPlatform/sapagent/shared/gce/metadataserver"
"github.com/GoogleCloudPlatform/sapagent/shared/log"
)

Expand Down Expand Up @@ -121,15 +122,21 @@ func TestStatusHandler(t *testing.T) {
exists: func(string) bool {
return true
},
fetchCloudProps: func() *metadataserver.CloudProperties {
return &metadataserver.CloudProperties{
Scopes: []string{"https://www.googleapis.com/auth/cloud-platform"},
}
},
},
want: &spb.AgentStatus{
AgentName: agentPackageName,
InstalledVersion: fmt.Sprintf("%s-%s", configuration.AgentVersion, configuration.AgentBuildChange),
AvailableVersion: "enabled",
SystemdServiceEnabled: spb.State_SUCCESS_STATE,
SystemdServiceRunning: spb.State_SUCCESS_STATE,
ConfigurationFilePath: configuration.LinuxConfigPath,
ConfigurationValid: spb.State_SUCCESS_STATE,
AgentName: agentPackageName,
InstalledVersion: fmt.Sprintf("%s-%s", configuration.AgentVersion, configuration.AgentBuildChange),
AvailableVersion: "enabled",
SystemdServiceEnabled: spb.State_SUCCESS_STATE,
SystemdServiceRunning: spb.State_SUCCESS_STATE,
ConfigurationFilePath: configuration.LinuxConfigPath,
ConfigurationValid: spb.State_SUCCESS_STATE,
CloudApiAccessFullScopesGranted: spb.State_SUCCESS_STATE,
Services: []*spb.ServiceStatus{
{
Name: "Host Metrics",
Expand Down Expand Up @@ -232,15 +239,21 @@ func TestStatusHandler(t *testing.T) {
exists: func(string) bool {
return false
},
fetchCloudProps: func() *metadataserver.CloudProperties {
return &metadataserver.CloudProperties{
Scopes: []string{},
}
},
},
want: &spb.AgentStatus{
AgentName: agentPackageName,
InstalledVersion: fmt.Sprintf("%s-%s", configuration.AgentVersion, configuration.AgentBuildChange),
AvailableVersion: fetchLatestVersionError,
SystemdServiceEnabled: spb.State_ERROR_STATE,
SystemdServiceRunning: spb.State_ERROR_STATE,
ConfigurationFilePath: configuration.LinuxConfigPath,
ConfigurationValid: spb.State_SUCCESS_STATE,
AgentName: agentPackageName,
InstalledVersion: fmt.Sprintf("%s-%s", configuration.AgentVersion, configuration.AgentBuildChange),
AvailableVersion: fetchLatestVersionError,
SystemdServiceEnabled: spb.State_ERROR_STATE,
SystemdServiceRunning: spb.State_ERROR_STATE,
ConfigurationFilePath: configuration.LinuxConfigPath,
ConfigurationValid: spb.State_SUCCESS_STATE,
CloudApiAccessFullScopesGranted: spb.State_FAILURE_STATE,
Services: []*spb.ServiceStatus{
{
Name: "Host Metrics",
Expand Down
1 change: 1 addition & 0 deletions protos/instanceinfo/instanceinfo.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ message CloudProperties {
string region = 7; // This is needed only for baremtal systems and is not
// used for GCE instances.
string machine_type = 8;
repeated string scopes = 9;
}

message Disk {
Expand Down
1 change: 1 addition & 0 deletions protos/status/status.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ message AgentStatus {
repeated ServiceStatus services = 8;
repeated Reference references = 9;
string agent_name = 10;
State cloud_api_access_full_scopes_granted = 11;
}

message ServiceStatus {
Expand Down
31 changes: 24 additions & 7 deletions shared/gce/metadataserver/metadataserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const (
MachineTypeUnknown = "unknown"
)

// GetCloudProperties abstracts metadataserver.FetchCloudProperties function for testability.
type GetCloudProperties func() *CloudProperties

var (
zonePattern = regexp.MustCompile("zones/([^/]*)")
machineTypePattern = regexp.MustCompile("machineTypes/([^/]*)")
Expand Down Expand Up @@ -73,16 +76,26 @@ type (
}

instanceInfo struct {
ID int64 `json:"id"`
Zone string `json:"zone"`
Name string `json:"name"`
Image string `json:"image"`
MachineType string `json:"machineType"`
ID int64 `json:"id"`
Zone string `json:"zone"`
Name string `json:"name"`
Image string `json:"image"`
MachineType string `json:"machineType"`
ServiceAccounts serviceAccounts `json:"serviceAccounts"`
}

serviceAccounts struct {
DefaultInfo defaultInfo `json:"default"`
}

defaultInfo struct {
Scopes []string `json:"scopes"`
}

// CloudProperties contains the cloud properties of the instance.
CloudProperties struct {
ProjectID, NumericProjectID, InstanceID, Zone, InstanceName, Image, MachineType string
Scopes []string
}
)

Expand Down Expand Up @@ -216,6 +229,7 @@ func requestCloudProperties() (*instancepb.CloudProperties, error) {
machineType := parseMachineType(instance.MachineType)
instanceName := instance.Name
image := instance.Image
scopes := instance.ServiceAccounts.DefaultInfo.Scopes

if image == "" {
image = ImageUnknown
Expand All @@ -225,7 +239,7 @@ func requestCloudProperties() (*instancepb.CloudProperties, error) {
}

log.Logger.Debugw("Default Cloud Properties from metadata server",
"projectid", projectID, "projectnumber", numericProjectID, "instanceid", instanceID, "zone", zone, "instancename", instanceName, "image", image, "machinetype", machineType)
"projectid", projectID, "projectnumber", numericProjectID, "instanceid", instanceID, "zone", zone, "instancename", instanceName, "image", image, "machinetype", machineType, "scopes", scopes)

if projectID == "" || numericProjectID == "0" || instanceID == "0" || zone == "" || instanceName == "" {
return nil, fmt.Errorf("metadata server responded with incomplete information")
Expand All @@ -239,6 +253,7 @@ func requestCloudProperties() (*instancepb.CloudProperties, error) {
InstanceName: instanceName,
Image: image,
MachineType: machineType,
Scopes: scopes,
}, nil
}

Expand All @@ -262,6 +277,7 @@ func requestProperties() (*CloudProperties, error) {
machineType := parseMachineType(instance.MachineType)
instanceName := instance.Name
image := instance.Image
scopes := instance.ServiceAccounts.DefaultInfo.Scopes

if image == "" {
image = ImageUnknown
Expand All @@ -271,7 +287,7 @@ func requestProperties() (*CloudProperties, error) {
}

log.Logger.Debugw("Default Cloud Properties from metadata server",
"projectid", projectID, "projectnumber", numericProjectID, "instanceid", instanceID, "zone", zone, "instancename", instanceName, "image", image, "machinetype", machineType)
"projectid", projectID, "projectnumber", numericProjectID, "instanceid", instanceID, "zone", zone, "instancename", instanceName, "image", image, "machinetype", machineType, "scopes", scopes)

if projectID == "" || numericProjectID == "0" || instanceID == "0" || zone == "" || instanceName == "" {
return nil, fmt.Errorf("metadata server responded with incomplete information")
Expand All @@ -285,6 +301,7 @@ func requestProperties() (*CloudProperties, error) {
InstanceName: instanceName,
Image: image,
MachineType: machineType,
Scopes: scopes,
}, nil
}

Expand Down
18 changes: 18 additions & 0 deletions shared/gce/metadataserver/metadataserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ func TestCloudPropertiesWithRetry(t *testing.T) {
Name: "test-instance-name",
Image: "test-image",
MachineType: "projects/test-project/machineTypes/test-machine-type",
ServiceAccounts: serviceAccounts{
DefaultInfo: defaultInfo{
Scopes: []string{"scope1", "scope2"},
},
},
},
})},
want: &instancepb.CloudProperties{
Expand All @@ -252,6 +257,7 @@ func TestCloudPropertiesWithRetry(t *testing.T) {
InstanceName: "test-instance-name",
Image: "test-image",
MachineType: "test-machine-type",
Scopes: []string{"scope1", "scope2"},
},
},
{
Expand Down Expand Up @@ -399,6 +405,11 @@ func TestReadCloudPropertiesWithRetry(t *testing.T) {
Name: "test-instance-name",
Image: "test-image",
MachineType: "projects/test-project/machineTypes/test-machine-type",
ServiceAccounts: serviceAccounts{
DefaultInfo: defaultInfo{
Scopes: []string{"scope1", "scope2"},
},
},
},
})},
want: &CloudProperties{
Expand All @@ -409,6 +420,7 @@ func TestReadCloudPropertiesWithRetry(t *testing.T) {
InstanceName: "test-instance-name",
Image: "test-image",
MachineType: "test-machine-type",
Scopes: []string{"scope1", "scope2"},
},
},
{
Expand Down Expand Up @@ -497,6 +509,11 @@ func TestFetchCloudProperties(t *testing.T) {
Name: "test-instance-name",
Image: "test-image",
MachineType: "projects/test-project/machineTypes/test-machine-type",
ServiceAccounts: serviceAccounts{
DefaultInfo: defaultInfo{
Scopes: []string{"scope1", "scope2"},
},
},
},
})}
want := &CloudProperties{
Expand All @@ -507,6 +524,7 @@ func TestFetchCloudProperties(t *testing.T) {
InstanceName: "test-instance-name",
Image: "test-image",
MachineType: "test-machine-type",
Scopes: []string{"scope1", "scope2"},
}

ts := mockMetadataServer(t, url)
Expand Down
7 changes: 7 additions & 0 deletions shared/statushelper/statushelper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package statushelper
import (
"context"
"fmt"
"slices"
"sort"
"strings"

Expand Down Expand Up @@ -149,6 +150,11 @@ func CheckIAMRoles(ctx context.Context, projectID string, requiredRoles []string
return nil
}

// CheckScopes validates that the scopes granted are equal to the required scopes.
func CheckScopes(ctx context.Context, scopes []string, requiredScopes []string) (hasRequiredScopes bool, err error) {
return slices.Equal(scopes, requiredScopes), nil
}

// packageVersionLinux returns the latest version of the agent package
// available on the linux OS's package manager.
func packageVersionLinux(ctx context.Context, packageName string, repoName string, exec commandlineexecutor.Execute, exists commandlineexecutor.Exists) (string, error) {
Expand Down Expand Up @@ -217,6 +223,7 @@ func PrintStatus(ctx context.Context, status *spb.AgentStatus) {
if status.GetConfigurationValid() != spb.State_SUCCESS_STATE {
printColor(failure, " %s\n", status.GetConfigurationErrorMessage())
}
printState(ctx, " Cloud API Access Full Scopes Granted", status.GetCloudApiAccessFullScopesGranted())

for _, service := range status.GetServices() {
printServiceStatus(ctx, service)
Expand Down
Loading

0 comments on commit 234eb5f

Please sign in to comment.