Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch description from query parameter to form data sending deploy description to dag server #1726

Merged
merged 11 commits into from
Oct 14, 2024
Merged
2 changes: 1 addition & 1 deletion cmd/software/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func NewDeployCmd() *cobra.Command {
cmd.Flags().BoolVarP(&saveDeployConfig, "save", "s", false, "Save deployment in config for future deploys")
cmd.Flags().BoolVarP(&ignoreCacheDeploy, "no-cache", "", false, "Do not use cache when building container image")
cmd.Flags().StringVar(&workspaceID, "workspace-id", "", "workspace assigned to deployment")
cmd.Flags().StringVar(&description, "description", "", "Reason for the deploy update")
cmd.Flags().StringVar(&description, "description", "", "Improve traceability by attaching a description to a code deploy. If you don't provide a description, the system automatically assigns a default description based on the deploy type.")

if !context.IsCloudContext() && houston.VerifyVersionMatch(houstonVersion, houston.VersionRestrictions{GTE: "0.34.0"}) {
cmd.Flags().BoolVarP(&isDagOnlyDeploy, "dags", "d", false, "Push only DAGs to your Deployment")
Expand Down
19 changes: 16 additions & 3 deletions pkg/fileutil/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type UploadFileArguments struct {
TargetURL string
FormFileFieldName string
Headers map[string]string
Description string
MaxTries int
InitialDelayInMS int
BackoffFactor int
Expand Down Expand Up @@ -310,6 +311,13 @@ func UploadFile(args *UploadFileArguments) error {
if err != nil {
return fmt.Errorf("error copying file content: %w", err)
}
// Add the description field to the multipart request
if args.Description != "" {
err = writer.WriteField("description", args.Description)
if err != nil {
return fmt.Errorf("error adding description field: %w", err)
}
}
// Close the multipart writer to finalize the request
writer.Close()

Expand All @@ -326,9 +334,8 @@ func UploadFile(args *UploadFileArguments) error {
}

// set headers
for k, v := range headers {
req.Header.Set(k, v)
}
setHeaders(req, headers)

client := &http.Client{}
response, err := client.Do(req)
Copy link
Member Author

@Simpcyclassy Simpcyclassy Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only tested for no description. I was unable to fully test the description. When logging the request body just before and after this line response, err := client.Do(req), I noticed that no data is logged after the request is sent.
Given this, simulating and testing for the description field and even file did not quite work.

if err != nil {
Expand Down Expand Up @@ -369,6 +376,12 @@ func UploadFile(args *UploadFileArguments) error {
return currentUploadError
}

func setHeaders(req *http.Request, headers map[string]string) {
for k, v := range headers {
req.Header.Set(k, v)
}
}

func GzipFile(srcFilePath, destFilePath string) error {
srcFile, err := os.Open(srcFilePath)
if err != nil {
Expand Down
67 changes: 57 additions & 10 deletions pkg/fileutil/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,23 +573,26 @@ func createMockServer(statusCode int, responseBody string, headers map[string][]
return httptest.NewServer(handler)
}

func getCapturedRequest(server *httptest.Server) *http.Request {
func getCapturedRequest(server *httptest.Server) ([]byte, http.Header) {
handler, ok := server.Config.Handler.(*testHandler)
if !ok {
panic("Unexpected server handler type")
}
return handler.Request
return handler.RequestBody, handler.RequestHeader
}

type testHandler struct {
StatusCode int
ResponseBody string
Headers map[string][]string
Request *http.Request
StatusCode int
ResponseBody string
Headers map[string][]string
RequestBody []byte
RequestHeader http.Header
}

func (h *testHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
h.Request = r
req, _ := io.ReadAll(r.Body)
h.RequestBody = req
h.RequestHeader = r.Header
w.WriteHeader(h.StatusCode)
for key, values := range h.Headers {
w.Header()[key] = values
Expand Down Expand Up @@ -632,6 +635,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: "http://localhost:8080/upload",
FormFileFieldName: "file",
Headers: map[string]string{},
Description: "Deployed via <astro deploy --dags>",
MaxTries: 3,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down Expand Up @@ -660,6 +664,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: "testURL",
FormFileFieldName: "file",
Headers: map[string]string{},
Description: "Deployed via <astro deploy --dags>",
MaxTries: 3,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down Expand Up @@ -688,6 +693,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: "testURL",
FormFileFieldName: "file",
Headers: map[string]string{},
Description: "Deployed via <astro deploy --dags>",
MaxTries: 3,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down Expand Up @@ -721,6 +727,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: testServer.URL,
FormFileFieldName: "file",
Headers: headers,
Description: "Deployed via <astro deploy --dags>",
MaxTries: 2,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down Expand Up @@ -756,6 +763,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: testServer.URL,
FormFileFieldName: "file",
Headers: headers,
Description: "Deployed via <astro deploy --dags>",
MaxTries: 2,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand All @@ -782,6 +790,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: "https://astro.unit.test",
FormFileFieldName: "file",
Headers: map[string]string{},
Description: "Deployed via <astro deploy --dags>",
MaxTries: 2,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down Expand Up @@ -814,6 +823,7 @@ func (s *Suite) TestUploadFile() {
TargetURL: server.URL,
FormFileFieldName: "file",
Headers: headers,
Description: "Deployed via <astro deploy --dags>",
MaxTries: 2,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand All @@ -823,8 +833,45 @@ func (s *Suite) TestUploadFile() {

s.NoError(err, "Expected no error")
// assert the received headers
request := getCapturedRequest(server)
s.Equal("Bearer token", request.Header.Get("Authorization"))
s.Contains(request.Header.Get("Content-Type"), "multipart/form-data")
reqBody, header := getCapturedRequest(server)
s.Equal("Bearer token", header.Get("Authorization"))
s.Contains(header.Get("Content-Type"), "multipart/form-data")
s.Contains(string(reqBody), "description")
s.Contains(string(reqBody), "Deployed via <astro deploy --dags>")
})

s.Run("successfully uploaded with an empty description", func() {
server := createMockServer(http.StatusOK, "OK", make(map[string][]string))
defer server.Close()

filePath := "./testFile.txt"
fileContent := []byte("This is a test file.")
err := os.WriteFile(filePath, fileContent, os.ModePerm)
s.NoError(err, "Error creating test file")
defer os.Remove(filePath)

headers := map[string]string{
"Authorization": "Bearer token",
"Content-Type": "application/json",
}

uploadFileArgs := UploadFileArguments{
FilePath: filePath,
TargetURL: server.URL,
FormFileFieldName: "file",
Headers: headers,
Description: "",
MaxTries: 2,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
RetryDisplayMessage: "please wait, attempting to upload the dags",
}
err = UploadFile(&uploadFileArgs)

s.NoError(err, "Expected no error")
reqBody, header := getCapturedRequest(server)
s.Equal("Bearer token", header.Get("Authorization"))
s.Contains(header.Get("Content-Type"), "multipart/form-data")
s.NotContains(string(reqBody), "description")
})
}
7 changes: 4 additions & 3 deletions software/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,9 @@ func validateIfDagDeployURLCanBeConstructed(deploymentInfo *houston.Deployment)
return nil
}

func getDagDeployURL(deploymentInfo *houston.Deployment, description string) string {
func getDagDeployURL(deploymentInfo *houston.Deployment) string {
c, _ := config.GetCurrentContext()
return fmt.Sprintf("https://deployments.%s/%s/dags/upload?description=%s", c.Domain, deploymentInfo.ReleaseName, description)
return fmt.Sprintf("https://deployments.%s/%s/dags/upload", c.Domain, deploymentInfo.ReleaseName)
}

func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.AppConfig, wsID, deploymentID, dagsParentPath string, dagDeployURL *string, cleanUpFiles bool, description string) error {
Expand Down Expand Up @@ -372,7 +372,7 @@ func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.Ap
if err != nil {
return err
}
uploadURL = getDagDeployURL(deploymentInfo, description)
uploadURL = getDagDeployURL(deploymentInfo)
} else {
uploadURL = *dagDeployURL
}
Expand Down Expand Up @@ -419,6 +419,7 @@ func DagsOnlyDeploy(houstonClient houston.ClientInterface, appConfig *houston.Ap
TargetURL: uploadURL,
FormFileFieldName: "file",
Headers: headers,
Description: description,
MaxTries: 8,
InitialDelayInMS: 1 * 1000,
BackoffFactor: 2,
Expand Down