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 d69d8e1
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 37 deletions.
10 changes: 10 additions & 0 deletions internal/onetime/status/status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"os"
"runtime"
"slices"

"flag"
"github.com/google/subcommands"
Expand All @@ -41,6 +42,7 @@ import (
const (
agentPackageName = "google-cloud-sap-agent"
fetchLatestVersionError = "Error: could not fetch latest version"
requiredScope = "https://www.googleapis.com/auth/cloud-platform"
)

// Status stores the status subcommand parameters.
Expand Down Expand Up @@ -148,6 +150,14 @@ func (s *Status) agentStatus(ctx context.Context) (*spb.AgentStatus, *cpb.Config
agentStatus.AvailableVersion = fetchLatestVersionError
}

agentStatus.CloudApiAccessFullScopesGranted = spb.State_FAILURE_STATE
if s.cloudProps == nil {
log.CtxLogger(ctx).Errorw("Could not fetch scopes", "error", err)
agentStatus.CloudApiAccessFullScopesGranted = spb.State_ERROR_STATE
} else if slices.Contains(s.cloudProps.GetScopes(), requiredScope) {
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
36 changes: 22 additions & 14 deletions internal/onetime/status/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,19 @@ func TestStatusHandler(t *testing.T) {
exists: func(string) bool {
return true
},
cloudProps: &ipb.CloudProperties{
Scopes: []string{requiredScope},
},
},
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 +236,19 @@ func TestStatusHandler(t *testing.T) {
exists: func(string) bool {
return false
},
cloudProps: &ipb.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
1 change: 1 addition & 0 deletions shared/statushelper/statushelper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,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
37 changes: 21 additions & 16 deletions shared/statushelper/statushelper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,21 +419,23 @@ Agent Status:
Configuration File:
Configuration Valid: Error: could not determine status
Cloud API Access Full Scopes Granted: Error: could not determine status
`,
},
{
name: "fullStatusAllSuccess",
status: &spb.AgentStatus{
AgentName: "Agent for SAP",
InstalledVersion: "3.6",
AvailableVersion: "3.6",
SystemdServiceEnabled: spb.State_SUCCESS_STATE,
SystemdServiceRunning: spb.State_SUCCESS_STATE,
ConfigurationFilePath: "/etc/google-cloud-sap-agent/configuration.json",
ConfigurationValid: spb.State_SUCCESS_STATE,
ConfigurationErrorMessage: "error: proto: (line 6:44): invalid value for bool field value: 2",
AgentName: "Agent for SAP",
InstalledVersion: "3.6",
AvailableVersion: "3.6",
SystemdServiceEnabled: spb.State_SUCCESS_STATE,
SystemdServiceRunning: spb.State_SUCCESS_STATE,
ConfigurationFilePath: "/etc/google-cloud-sap-agent/configuration.json",
ConfigurationValid: spb.State_SUCCESS_STATE,
ConfigurationErrorMessage: "error: proto: (line 6:44): invalid value for bool field value: 2",
CloudApiAccessFullScopesGranted: spb.State_SUCCESS_STATE,
Services: []*spb.ServiceStatus{
{
Name: "Process Metrics",
Expand Down Expand Up @@ -511,6 +513,7 @@ Agent Status:
Systemd Service Running: True
Configuration File: /etc/google-cloud-sap-agent/configuration.json
Configuration Valid: True
Cloud API Access Full Scopes Granted: True
--------------------------------------------------------------------------------
Process Metrics: Enabled
Status: Fully Functional
Expand All @@ -536,14 +539,15 @@ What's New: https://cloud.google.com/solutions/sap/docs/agent-for-sap/whats-new
{
name: "fullStatusWithFailures",
status: &spb.AgentStatus{
AgentName: "Agent for SAP",
InstalledVersion: "3.5",
AvailableVersion: "3.6",
SystemdServiceEnabled: spb.State_FAILURE_STATE,
SystemdServiceRunning: spb.State_FAILURE_STATE,
ConfigurationFilePath: "/etc/google-cloud-sap-agent/configuration.json",
ConfigurationValid: spb.State_FAILURE_STATE,
ConfigurationErrorMessage: "error: proto: (line 6:44): invalid value for bool field value: 2",
AgentName: "Agent for SAP",
InstalledVersion: "3.5",
AvailableVersion: "3.6",
SystemdServiceEnabled: spb.State_FAILURE_STATE,
SystemdServiceRunning: spb.State_FAILURE_STATE,
ConfigurationFilePath: "/etc/google-cloud-sap-agent/configuration.json",
ConfigurationValid: spb.State_FAILURE_STATE,
ConfigurationErrorMessage: "error: proto: (line 6:44): invalid value for bool field value: 2",
CloudApiAccessFullScopesGranted: spb.State_FAILURE_STATE,
Services: []*spb.ServiceStatus{
{
Name: "Process Metrics",
Expand Down Expand Up @@ -620,6 +624,7 @@ Agent Status:
Configuration File: /etc/google-cloud-sap-agent/configuration.json
Configuration Valid: False
error: proto: (line 6:44): invalid value for bool field value: 2
Cloud API Access Full Scopes Granted: False
--------------------------------------------------------------------------------
Process Metrics: Enabled
Status: Error: Cannot write to Cloud Monitoring, check IAM permissions
Expand Down

0 comments on commit d69d8e1

Please sign in to comment.