Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-db-changes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:

- name: Start Infra Dependencies
# Use & to background this
run: docker-compose up -d mysql_test &
run: docker compose up -d mysql_test &

- name: Verify test schema changes
run: |
Expand Down
1 change: 1 addition & 0 deletions changes/20271-deleted-host-software-installs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fig bug where software install results could not be retrieved for deleted hosts in the activity feed
34 changes: 30 additions & 4 deletions ee/server/service/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,38 @@ func (svc *Service) GetSoftwareInstallResults(ctx context.Context, resultUUID st

res, err := svc.ds.GetSoftwareInstallResults(ctx, resultUUID)
if err != nil {
return nil, err
if fleet.IsNotFound(err) {
if err := svc.authz.Authorize(ctx, &fleet.HostSoftwareInstallerResultAuthz{}, fleet.ActionRead); err != nil {
return nil, err
}
}
svc.authz.SkipAuthorization(ctx)
return nil, ctxerr.Wrap(ctx, err, "get software install result")
}

// Team specific auth check
if err := svc.authz.Authorize(ctx, &fleet.HostSoftwareInstallerResultAuthz{HostTeamID: res.HostTeamID}, fleet.ActionRead); err != nil {
return nil, err
if res.HostDeletedAt == nil {
// host is not deleted, get it and authorize for the host's team
host, err := svc.ds.HostLite(ctx, res.HostID)
// if error is because the host does not exist, check first if the user
// had access to run a script (to prevent leaking valid host ids).
if err != nil {
if fleet.IsNotFound(err) {
if err := svc.authz.Authorize(ctx, &fleet.HostSoftwareInstallerResultAuthz{}, fleet.ActionRead); err != nil {
return nil, err
}
}
svc.authz.SkipAuthorization(ctx)
return nil, ctxerr.Wrap(ctx, err, "get host lite")
}
// Team specific auth check
if err := svc.authz.Authorize(ctx, &fleet.HostSoftwareInstallerResultAuthz{HostTeamID: host.TeamID}, fleet.ActionRead); err != nil {
return nil, err
}
} else {
// host was deleted, authorize for no-team as a fallback
if err := svc.authz.Authorize(ctx, &fleet.HostSoftwareInstallerResultAuthz{}, fleet.ActionRead); err != nil {
return nil, err
}
}

res.EnhanceOutputDetails()
Expand Down
5 changes: 3 additions & 2 deletions server/datastore/mysql/hosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,8 @@ var additionalHostRefsByUUID = map[string]string{
// the rows are not deleted when the host is deleted, only a soft delete is
// performed by setting a timestamp column to the current time.
var additionalHostRefsSoftDelete = map[string]string{
"host_script_results": "host_deleted_at",
"host_script_results": "host_deleted_at",
"host_software_installs": "host_deleted_at",
}

func (ds *Datastore) DeleteHost(ctx context.Context, hid uint) error {
Expand Down Expand Up @@ -4943,7 +4944,7 @@ func (ds *Datastore) ListUpcomingHostMaintenanceWindows(ctx context.Context, hid
WHERE
hce.host_id = ?
AND
ce.start_time > NOW()
ce.start_time > NOW()
ORDER BY ce.start_time
`

Expand Down
9 changes: 9 additions & 0 deletions server/datastore/mysql/hosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6743,6 +6743,15 @@ func testHostsDeleteHosts(t *testing.T, ds *Datastore) {
`, host.ID, calendarEventID)
require.NoError(t, err)

softwareInstaller, err := ds.MatchOrCreateSoftwareInstaller(context.Background(), &fleet.UploadSoftwareInstallerPayload{
InstallScript: "",
PreInstallQuery: "",
Title: "ChocolateRain",
})
require.NoError(t, err)
_, err = ds.InsertSoftwareInstallRequest(context.Background(), host.ID, softwareInstaller, false)
require.NoError(t, err)

// Check there's an entry for the host in all the associated tables.
for _, hostRef := range hostRefs {
var ok bool
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package tables

import (
"database/sql"
"fmt"
)

func init() {
MigrationClient.AddMigration(Up_20240802101043, Down_20240802101043)
}

// This is a new copy of a previous out-of-order migration
func Up_20240802101043(tx *sql.Tx) error {
_, err := tx.Exec("ALTER TABLE host_software_installs ADD COLUMN host_deleted_at timestamp NULL DEFAULT NULL")
if err != nil {
return fmt.Errorf("failed to create host_deleted_at column on host_software_installs table: %w", err)
}
_, err = tx.Exec(`
UPDATE
host_software_installs i
LEFT JOIN
hosts h
ON i.host_id = h.id
SET
i.host_deleted_at = NOW()
WHERE
i.host_deleted_at IS NULL
AND
h.id IS NULL
`)
if err != nil {
return fmt.Errorf("failed to update host_software_installs.host_deleted_at for hosts that no longer exist: %w", err)
}
return nil
}

func Down_20240802101043(tx *sql.Tx) error {
return nil
}
5 changes: 3 additions & 2 deletions server/datastore/mysql/schema.sql

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions server/datastore/mysql/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,19 +337,17 @@ SELECT
hsi.post_install_script_output,
hsi.install_script_output,
hsi.host_id AS host_id,
h.computer_name AS host_display_name,
st.name AS software_title,
st.id AS software_title_id,
COALESCE(%s, '') AS status,
si.filename AS software_package,
h.team_id AS host_team_id,
hsi.user_id AS user_id,
hsi.post_install_script_exit_code,
hsi.install_script_exit_code,
hsi.self_service
hsi.self_service,
hsi.host_deleted_at
FROM
host_software_installs hsi
JOIN hosts h ON h.id = hsi.host_id
JOIN software_installers si ON si.id = hsi.software_installer_id
JOIN software_titles st ON si.title_id = st.id
WHERE
Expand Down Expand Up @@ -400,6 +398,7 @@ WHERE
FROM host_software_installs
WHERE
software_installer_id = :installer_id
AND host_deleted_at IS NULL
GROUP BY
host_id)) s`, softwareInstallerHostStatusNamedQuery("hsi", "status"))

Expand Down
1 change: 0 additions & 1 deletion server/datastore/mysql/software_installers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ func testGetSoftwareInstallResult(t *testing.T, ds *Datastore) {
require.Equal(t, tc.expectedStatus, res.Status)
require.Equal(t, swFilename, res.SoftwarePackage)
require.Equal(t, host.ID, res.HostID)
require.Equal(t, host.DisplayName(), res.HostDisplayName)
require.Equal(t, tc.preInstallQueryOutput, res.PreInstallQueryOutput)
require.Equal(t, tc.postInstallScriptOutput, res.PostInstallScriptOutput)
require.Equal(t, tc.installScriptOutput, res.Output)
Expand Down
8 changes: 3 additions & 5 deletions server/fleet/software_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ type HostSoftwareInstallerResult struct {
SoftwarePackage string `json:"software_package" db:"software_package"`
// HostID is the ID of the host.
HostID uint `json:"host_id" db:"host_id"`
// HostDisplayName is the display name of the host.
HostDisplayName string `json:"host_display_name" db:"host_display_name"`
// Status is the status of the software installer package on the host.
Status SoftwareInstallerStatus `json:"status" db:"status"`
// Detail is the detail of the software installer package on the host. TODO: does this field
Expand All @@ -172,9 +170,6 @@ type HostSoftwareInstallerResult struct {
CreatedAt time.Time `json:"created_at" db:"created_at"`
// UpdatedAt is the time the software installer request was last updated.
UpdatedAt *time.Time `json:"updated_at" db:"updated_at"`
// HostTeamID is the team ID of the host on which this software install was attempted. This
// field is not sent in the response, it is only used for internal authorization.
HostTeamID *uint `json:"-" db:"host_team_id"`
// UserID is the user ID that requested the software installation on that host.
UserID *uint `json:"-" db:"user_id"`
// InstallScriptExitCode is used internally to determine the output displayed to the user.
Expand All @@ -184,6 +179,9 @@ type HostSoftwareInstallerResult struct {
// SelfService indicates that the installation was queued by the
// end user and not an administrator
SelfService bool `json:"self_service" db:"self_service"`
// HostDeletedAt indicates if the data is associated with a
// deleted host
HostDeletedAt *time.Time `json:"-" db:"host_deleted_at"`
}

const (
Expand Down
16 changes: 16 additions & 0 deletions server/service/integration_enterprise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10619,6 +10619,22 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerHostRequests() {
require.Contains(t, extractServerErrorText(r.Body), "Invalid parameters. The combination of software_version_id and software_title_id is not allowed.")
r = s.Do("GET", "/api/latest/fleet/hosts", nil, http.StatusBadRequest, "software_status", "installed", "team_id", "1", "software_title_id", "1", "software_id", "1")
require.Contains(t, extractServerErrorText(r.Body), "Invalid parameters. The combination of software_id and software_title_id is not allowed.")

// Access software install result after host is deleted
err = s.ds.DeleteHost(context.Background(), h.ID)
require.NoError(t, err)

instResult, err := s.ds.GetSoftwareInstallResults(context.Background(), installUUID)
require.NoError(t, err)
require.NotNil(t, instResult.HostDeletedAt)

gsirr = getSoftwareInstallResultsResponse{}
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/install/results/%s", installUUID), nil, http.StatusOK, &gsirr)
require.NoError(t, gsirr.Err)
require.NotNil(t, gsirr.Results)
results = gsirr.Results
require.Equal(t, installUUID, results.InstallUUID)
require.Equal(t, fleet.SoftwareInstallerPending, results.Status)
}

func (s *integrationEnterpriseTestSuite) TestSelfServiceSoftwareInstall() {
Expand Down
2 changes: 1 addition & 1 deletion tools/dbutils/schema_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func main() {

// Dump schema to dumpfile
cmd := exec.Command(
"docker-compose", "exec", "-T", "mysql_test",
"docker", "compose", "exec", "-T", "mysql_test",
// Command run inside container
"mysqldump", "-u"+testUsername, "-p"+testPassword, "schemadb", "--compact", "--skip-comments",
)
Expand Down