Skip to content

Commit

Permalink
Bump max length for installer URLs supplied in GitOps to 4000 charact…
Browse files Browse the repository at this point in the history
…ers (#24942)

For #24917. Should be worth the extra byte per row for the varchar
field.

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated tests
- [x] If database migrations are included, checked table schema to
confirm autoupdate
- For database migrations:
- [x] Checked schema for all modified table for columns that will
auto-update timestamps during migration.
- [x] Confirmed that updating the timestamps is acceptable, and will not
cause unwanted side effects.
- [x] Ensured the correct collation is explicitly set for character
columns (`COLLATE utf8mb4_unicode_ci`).
- [x] Manual QA for all new/changed functionality
  • Loading branch information
iansltx authored Dec 20, 2024
1 parent ccb44a3 commit 1f39717
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 9 deletions.
1 change: 1 addition & 0 deletions changes/24917-installer-url-length
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Increased maximum length for installer URLs specified in GitOps to 4000 characters
2 changes: 1 addition & 1 deletion ee/server/service/software_installers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ func (svc *Service) BatchSetSoftwareInstallers(
if len(payload.URL) > fleet.SoftwareInstallerURLMaxLength {
return "", fleet.NewInvalidArgumentError(
"software.url",
"software URL is too long, must be less than 256 characters",
fmt.Sprintf("software URL is too long, must be %d characters or less", fleet.SoftwareInstallerURLMaxLength),
)
}
if _, err := url.ParseRequestURI(payload.URL); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/spec/gitops.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin
continue
}
if len(softwarePackageSpec.URL) > fleet.SoftwareInstallerURLMaxLength {
multiError = multierror.Append(multiError, fmt.Errorf("software URL %q is too long, must be less than 256 characters", softwarePackageSpec.URL))
multiError = multierror.Append(multiError, fmt.Errorf("software URL %q is too long, must be %d characters or less", softwarePackageSpec.URL, fleet.SoftwareInstallerURLMaxLength))
continue
}
result.Software.Packages = append(result.Software.Packages, &softwarePackageSpec)
Expand Down
4 changes: 2 additions & 2 deletions pkg/spec/gitops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ policies:
assert.ErrorContains(t, err, "empty package_path")

// Software has a URL that's too big
tooBigURL := fmt.Sprintf("https://ftp.mozilla.org/%s", strings.Repeat("a", 232))
tooBigURL := fmt.Sprintf("https://ftp.mozilla.org/%s", strings.Repeat("a", 4000-23))
config = getTeamConfig([]string{"software"})
config += fmt.Sprintf(`
software:
Expand All @@ -922,7 +922,7 @@ software:
}
path, basePath := createTempFile(t, "", config)
_, err = GitOpsFromFile(path, basePath, &appConfig, nopLogf)
assert.ErrorContains(t, err, fmt.Sprintf("software URL \"%s\" is too long, must be less than 256 characters", tooBigURL))
assert.ErrorContains(t, err, fmt.Sprintf("software URL \"%s\" is too long, must be 4000 characters or less", tooBigURL))

// Policy references a software installer not present in the team.
config = getTeamConfig([]string{"policies"})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package tables

import (
"database/sql"
"fmt"
)

func init() {
MigrationClient.AddMigration(Up_20241219180042, Down_20241219180042)
}

func Up_20241219180042(tx *sql.Tx) error {
// The new 'url' column will only be set for software uploaded in batch via GitOps.
if _, err := tx.Exec(`
ALTER TABLE software_installers
CHANGE COLUMN url url VARCHAR(4095) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '';
`); err != nil {
return fmt.Errorf("failed to lengthen url in software_installers: %w", err)
}
return nil
}

func Down_20241219180042(tx *sql.Tx) error {
return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package tables

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestUp_20241219180042(t *testing.T) {
db := applyUpToPrev(t)

script1 := execNoErrLastID(t, db, "INSERT INTO script_contents(contents, md5_checksum) VALUES ('echo hi', 'a')")
script2 := execNoErrLastID(t, db, "INSERT INTO script_contents(contents, md5_checksum) VALUES ('echo bye', 'b')")

software := execNoErrLastID(t, db, `
INSERT INTO software_installers (
filename,
version,
platform,
install_script_content_id,
post_install_script_content_id,
uninstall_script_content_id,
storage_id,
package_ids,
url
) VALUES (
'fleet',
'1.0.0',
'windows',
?,
?,
?,
'a',
'',
?
)`, script1, script2, script2, "https://google.com/")

applyNext(t, db)

var url string
err := db.Get(&url, "SELECT url FROM software_installers WHERE id = ?", software)
require.NoError(t, err)
require.Equal(t, "https://google.com/", url)

longUrl := "https://dl.google.com/tag/s/appguid%3D%7B8A69D345-D564-463C-AFF1-A69D9E530F96%7D%26iid%3D%7B53CCDE8D-FD40-46DE-67E7-61E96CFEFCAA%7D%26lang%3Den%26browser%3D4%26usagestats%3D0%26appname%3DGoogle%2520Chrome%26needsadmin%3Dtrue%26ap%3Dx64-stable-statsdef_0%26brand%3DGCEA/dl/chrome/install/googlechromestandaloneenterprise64.msi"
execNoErr(t, db, `UPDATE software_installers SET url = ? WHERE id = ?`, longUrl, software)

err = db.Get(&url, "SELECT url FROM software_installers WHERE id = ?", software)
require.NoError(t, err)
require.Equal(t, longUrl, url)
}
6 changes: 3 additions & 3 deletions server/datastore/mysql/schema.sql

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion server/fleet/software_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ type SoftwareInstallerTokenMetadata struct {
TeamID uint `json:"team_id"`
}

const SoftwareInstallerURLMaxLength = 255
const SoftwareInstallerURLMaxLength = 4000

// TempFileReader is an io.Reader with all extra io interfaces supported by a
// file on disk reader (e.g. io.ReaderAt, io.Seeker, etc.). When created with
Expand Down
2 changes: 1 addition & 1 deletion server/service/integration_enterprise_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11237,7 +11237,7 @@ func (s *integrationEnterpriseTestSuite) TestBatchSetSoftwareInstallers() {

// software with a too big URL
softwareToInstall = []fleet.SoftwareInstallerPayload{
{URL: "https://ftp.mozilla.org/" + strings.Repeat("a", 233)},
{URL: "https://ftp.mozilla.org/" + strings.Repeat("a", 4000-23)},
}
s.Do("POST", "/api/latest/fleet/software/batch", batchSetSoftwareInstallersRequest{Software: softwareToInstall}, http.StatusUnprocessableEntity, "team_name", tm.Name)

Expand Down

0 comments on commit 1f39717

Please sign in to comment.