Always embed URL provided by user in the archive (#1413)

To keep archive creation implementation simple and prevent any
confusion, we decided to remove `--keepurl` flag and embed URL
directly without downloading the file from it . In this way, we can
ensure consistent behavior in either package creation or spec file
creation. Also, it increases the portability of spec file.

pkg/fission-cli/cmd/function/command.go

@@ -38,7 +38,7 @@ func Commands() *cobra.Command {
 
 			// TODO retired pkg related flag from function cmd
 			flag.FnCode, flag.PkgSrcArchive, flag.PkgDeployArchive,
-			flag.HtUrl, flag.HtMethod, flag.FnKeepURL,
+			flag.HtUrl, flag.HtMethod,
 
 			// flag for newdeploy to use.
 			flag.RunTimeMinCPU, flag.RunTimeMaxCPU, flag.RunTimeMinMemory,
@@ -84,7 +84,7 @@ func Commands() *cobra.Command {
 			flag.FnSpecializationTimeout, flag.FnExecutionTimeout,
 
 			flag.FnCode, flag.PkgSrcArchive, flag.PkgDeployArchive,
-			flag.FnKeepURL, flag.FnBuildCmd, flag.PkgForce,
+			flag.FnBuildCmd, flag.PkgForce,
 
 			flag.RunTimeMinCPU, flag.RunTimeMaxCPU, flag.RunTimeMinMemory,
 			flag.RunTimeMaxMemory, flag.ReplicasMin, flag.ReplicasMax,

pkg/fission-cli/cmd/function/create.go

@@ -169,11 +169,11 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
 		}
 
 		buildcmd := input.String(flagkey.PkgBuildCmd)
-		keepURL := input.Bool(flagkey.PkgKeepURL)
+		pkgName := fmt.Sprintf("%v-%v", fnName, uuid.NewV4().String())
 
 		// create new package in the same namespace as the function.
-		pkgMetadata, err = _package.CreatePackage(input, opts.client, fnName, fnNamespace, envName, envNamespace,
+		pkgMetadata, err = _package.CreatePackage(input, opts.client, pkgName, fnNamespace, envName, envNamespace,
-			srcArchiveFiles, deployArchiveFiles, buildcmd, specDir, opts.specFile, noZip, keepURL)
+			srcArchiveFiles, deployArchiveFiles, buildcmd, specDir, opts.specFile, noZip)
 		if err != nil {
 			return errors.Wrap(err, "error creating package")
 		}

pkg/fission-cli/cmd/function/update.go

@@ -226,9 +226,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
 			return errors.New("package is used by multiple functions, use --force to force update")
 		}
 
-		keepURL := input.Bool(flagkey.PkgKeepURL)
+		pkgMetadata, err = _package.UpdatePackage(opts.client, pkg, envName, envNamespace, srcArchiveFiles, deployArchiveFiles, buildcmd, false, codeFlag)
-
-		pkgMetadata, err = _package.UpdatePackage(opts.client, pkg, envName, envNamespace, srcArchiveFiles, deployArchiveFiles, buildcmd, false, codeFlag, keepURL)
 		if err != nil {
 			return errors.Wrap(err, fmt.Sprintf("error updating package '%v'", pkgName))
 		}

pkg/fission-cli/cmd/package/command.go

@@ -32,7 +32,7 @@ func Commands() *cobra.Command {
 	wrapper.SetFlags(createCmd, flag.FlagSet{
 		Required: []flag.Flag{flag.PkgEnvironment},
 		Optional: []flag.Flag{flag.PkgName, flag.PkgSrcArchive, flag.PkgDeployArchive,
-			flag.PkgKeepURL, flag.PkgBuildCmd, flag.NamespacePackage, flag.NamespaceEnvironment},
+			flag.PkgBuildCmd, flag.NamespacePackage, flag.NamespaceEnvironment},
 	})
 
 	getSrcCmd := &cobra.Command{
@@ -63,7 +63,7 @@ func Commands() *cobra.Command {
 	wrapper.SetFlags(updateCmd, flag.FlagSet{
 		Required: []flag.Flag{flag.PkgName},
 		Optional: []flag.Flag{flag.PkgEnvironment, flag.PkgSrcArchive, flag.PkgDeployArchive,
-			flag.PkgKeepURL, flag.PkgBuildCmd, flag.PkgForce, flag.NamespacePackage, flag.NamespaceEnvironment},
+			flag.PkgBuildCmd, flag.PkgForce, flag.NamespacePackage, flag.NamespaceEnvironment},
 	})
 
 	deleteCmd := &cobra.Command{

pkg/fission-cli/cmd/package/create.go

@@ -70,21 +70,20 @@ func (opts *CreateSubCommand) run(input cli.Input) error {
 	srcArchiveFiles := input.StringSlice(flagkey.PkgSrcArchive)
 	deployArchiveFiles := input.StringSlice(flagkey.PkgDeployArchive)
 	buildcmd := input.String(flagkey.PkgBuildCmd)
-	keepURL := input.Bool(flagkey.PkgKeepURL)
 
 	if len(srcArchiveFiles) == 0 && len(deployArchiveFiles) == 0 {
 		return errors.Errorf("need --%v or --%v flag", flagkey.PkgSrcArchive, flagkey.PkgDeployArchive)
 	}
 
 	_, err := CreatePackage(input, opts.client, pkgName, pkgNamespace, envName, envNamespace,
-		srcArchiveFiles, deployArchiveFiles, buildcmd, "", "", false, keepURL)
+		srcArchiveFiles, deployArchiveFiles, buildcmd, "", "", false)
 
 	return err
 }
 
 // TODO: get all necessary value from CLI input directly
 func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNamespace string, envName string, envNamespace string,
-	srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, specDir string, specFile string, noZip bool, keepURL bool) (*metav1.ObjectMeta, error) {
+	srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, specDir string, specFile string, noZip bool) (*metav1.ObjectMeta, error) {
 
 	pkgSpec := fv1.PackageSpec{
 		Environment: fv1.EnvironmentReference{
@@ -98,7 +97,7 @@ func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNa
 		if len(specFile) > 0 { // we should do this in all cases, i think
 			pkgStatus = fv1.BuildStatusNone
 		}
-		deployment, err := CreateArchive(client, deployArchiveFiles, noZip, keepURL, specDir, specFile)
+		deployment, err := CreateArchive(client, deployArchiveFiles, noZip, specDir, specFile)
 		if err != nil {
 			return nil, err
 		}
@@ -108,7 +107,7 @@ func CreatePackage(input cli.Input, client *client.Client, pkgName string, pkgNa
 		}
 	}
 	if len(srcArchiveFiles) > 0 {
-		source, err := CreateArchive(client, srcArchiveFiles, false, keepURL, specDir, specFile)
+		source, err := CreateArchive(client, srcArchiveFiles, false, specDir, specFile)
 		if err != nil {
 			return nil, err
 		}

pkg/fission-cli/cmd/package/package.go

@@ -40,7 +40,7 @@ import (
 // create an archive upload spec in the specs directory; otherwise
 // upload the archive using client.  noZip avoids zipping the
 // includeFiles, but is ignored if there's more than one includeFile.
-func CreateArchive(client *client.Client, includeFiles []string, noZip bool, keepURL bool, specDir string, specFile string) (*fv1.Archive, error) {
+func CreateArchive(client *client.Client, includeFiles []string, noZip bool, specDir string, specFile string) (*fv1.Archive, error) {
 	errs := utils.MultiErrorWithFormat()
 	fileURL := ""
 
@@ -49,8 +49,7 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee
 		// ignore http files
 		if utils.IsURL(path) {
 			if len(includeFiles) > 1 {
-				// It's intentional to disallow the user to provide file
+				// It's intentional to disallow the user to provide file and URL at the same time.
-				// and URL at the same time even the keepurl is false.
 				return nil, errors.New("unable to create an archive that contains both file and URL")
 			}
 			fileURL = path
@@ -73,13 +72,11 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee
 	}
 
 	if len(specFile) > 0 {
-		var archive fv1.Archive
-
 		if len(fileURL) > 0 {
-			archive = fv1.Archive{
+			return &fv1.Archive{
 				Type: fv1.ArchiveTypeUrl,
 				URL:  fileURL,
-			}
+			}, nil
 		} else {
 			// create an ArchiveUploadSpec and reference it from the archive
 			aus := &spectypes.ArchiveUploadSpec{
@@ -102,30 +99,20 @@ func CreateArchive(client *client.Client, includeFiles []string, noZip bool, kee
 					return nil, errors.Wrapf(err, "write spec file %v", specFile)
 				}
 			}
-
 			// create the archive object
-			archive = fv1.Archive{
+			archive := fv1.Archive{
 				Type: fv1.ArchiveTypeUrl,
 				URL:  fmt.Sprintf("%v%v", spec.ARCHIVE_URL_PREFIX, aus.Name),
 			}
+			return &archive, nil
 		}
-
-		return &archive, nil
 	}
 
 	if len(fileURL) > 0 {
-		if keepURL {
+		return &fv1.Archive{
-			return &fv1.Archive{
+			Type: fv1.ArchiveTypeUrl,
-				Type: fv1.ArchiveTypeUrl,
+			URL:  fileURL,
-				URL:  fileURL,
+		}, nil
-			}, nil
-		}
-		// download the file before we archive it
-		dst, err := pkgutil.DownloadToTempFile(fileURL)
-		if err != nil {
-			return nil, err
-		}
-		includeFiles = []string{dst}
 	}
 
 	archivePath, err := makeArchiveFile("", includeFiles, noZip)

pkg/fission-cli/cmd/package/update.go

@@ -40,7 +40,6 @@ type UpdateSubCommand struct {
 	srcArchiveFiles    []string
 	deployArchiveFiles []string
 	buildcmd           string
-	keepURL            bool
 }
 
 func Update(input cli.Input) error {
@@ -71,7 +70,6 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
 	opts.srcArchiveFiles = input.StringSlice(flagkey.PkgSrcArchive)
 	opts.deployArchiveFiles = input.StringSlice(flagkey.PkgDeployArchive)
 	opts.buildcmd = input.String(flagkey.PkgBuildCmd)
-	opts.keepURL = input.Bool(flagkey.PkgKeepURL)
 	return nil
 }
 
@@ -106,7 +104,7 @@ func (opts *UpdateSubCommand) run(input cli.Input) error {
 
 	newPkgMeta, err := UpdatePackage(opts.client, pkg,
 		opts.envName, opts.envNamespace, opts.srcArchiveFiles,
-		opts.deployArchiveFiles, opts.buildcmd, false, false, opts.keepURL)
+		opts.deployArchiveFiles, opts.buildcmd, false, false)
 	if err != nil {
 		return errors.Wrap(err, "update package")
 	}
@@ -125,7 +123,7 @@ func (opts *UpdateSubCommand) run(input cli.Input) error {
 }
 
 func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespace string,
-	srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, forceRebuild bool, noZip bool, keepURL bool) (*metav1.ObjectMeta, error) {
+	srcArchiveFiles []string, deployArchiveFiles []string, buildcmd string, forceRebuild bool, noZip bool) (*metav1.ObjectMeta, error) {
 
 	needToBuild := false
 
@@ -145,7 +143,7 @@ func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespac
 	}
 
 	if len(srcArchiveFiles) > 0 {
-		srcArchive, err := CreateArchive(client, srcArchiveFiles, false, keepURL, "", "")
+		srcArchive, err := CreateArchive(client, srcArchiveFiles, false, "", "")
 		if err != nil {
 			return nil, err
 		}
@@ -154,7 +152,7 @@ func UpdatePackage(client *client.Client, pkg *fv1.Package, envName, envNamespac
 	}
 
 	if len(deployArchiveFiles) > 0 {
-		deployArchive, err := CreateArchive(client, deployArchiveFiles, noZip, keepURL, "", "")
+		deployArchive, err := CreateArchive(client, deployArchiveFiles, noZip, "", "")
 		if err != nil {
 			return nil, err
 		}

pkg/fission-cli/flag/flag.go

@@ -89,7 +89,6 @@ var (
 	FnSpecializationTimeout = Flag{Type: Int, Name: flagkey.FnSpecializationTimeout, Aliases: []string{"st"}, Usage: "Timeout for executor to wait for function pod creation", DefaultValue: fv1.DefaultSpecializationTimeOut}
 	FnEnvName               = Flag{Type: String, Name: flagkey.FnEnvironmentName, Usage: "Environment name for function"}
 	FnCode                  = Flag{Type: String, Name: flagkey.FnCode, Usage: "Local path or URL for single file source code"}
-	FnKeepURL               = Flag{Type: Bool, Name: flagkey.PkgKeepURL, Aliases: []string{"keepurl"}, Usage: "Keep the providing URL in archive instead of downloading file from it. (If set, no checksum will be generated for file integrity check. You must ensure the file won't be changed.)"}
 	FnPkgName               = Flag{Type: String, Name: flagkey.FnPackageName, Aliases: []string{"pkg"}, Usage: "Name of the existing package (--deploy and --src and --env will be ignored), should be in the same namespace as the function"}
 	FnEntryPoint            = Flag{Type: String, Name: flagkey.FnEntrypoint, Aliases: []string{"entry"}, Usage: "Entry point for environment v2 to load with"}
 	FnBuildCmd              = Flag{Type: String, Name: flagkey.FnBuildCmd, Usage: "Package build command for builder to run with"}
@@ -153,7 +152,6 @@ var (
 	PkgName          = Flag{Type: String, Name: flagkey.PkgName, Usage: "Package name"}
 	PkgForce         = Flag{Type: Bool, Name: flagkey.PkgForce, Short: "f", Usage: "Force update a package even if it is used by one or more functions"}
 	PkgEnvironment   = Flag{Type: String, Name: flagkey.PkgEnvironment, Usage: "Environment name"}
-	PkgKeepURL       = Flag{Type: Bool, Name: flagkey.PkgKeepURL, Aliases: []string{"keepurl"}, Usage: "Keep the providing URL in archive instead of downloading file from it. (If set, no checksum will be generated for file integrity check. You must ensure the file won't be changed.)"}
 	PkgBuildCmd      = Flag{Type: String, Name: flagkey.PkgBuildCmd, Usage: "Build command for builder to run with"}
 	PkgOutput        = Flag{Type: String, Name: flagkey.PkgOutput, Short: "o", Usage: "Output filename to save archive content"}
 	PkgStatus        = Flag{Type: String, Name: flagkey.PkgStatus, Usage: `Filter packages by status`}

pkg/fission-cli/flag/key/key.go

@@ -109,7 +109,6 @@ const (
 	PkgEnvironment   = "env"
 	PkgSrcArchive    = "sourcearchive"
 	PkgDeployArchive = "deployarchive"
-	PkgKeepURL       = "keeparchiveurl"
 	PkgBuildCmd      = "buildcmd"
 	PkgOutput        = Output
 	PkgStatus        = "status"

test/tests/test_create_fn_with_url.sh

@@ -51,21 +51,4 @@ response=$(curl --retry 5 http://$FISSION_ROUTER/$fn)
 log "Checking for valid response"
 echo $response | grep -i hello
 
-log "Update function with file URL"
-fission fn update --name $fn --env $env --code ${code_url} --keepurl
-
-pkg=$(kubectl -n default get functions ${fn} -o yaml|grep hello-js|awk '{print $2}')
-url=$(kubectl -n default get packages ${pkg} -o yaml|grep "://"|awk '{print $2}')
-
-if [ ${url} != ${code_url} ]; then
-    log "have different code url: ${url} vs. ${code_url}"
-    exit 1
-fi
-
-log "Doing an HTTP GET on the function's route"
-response=$(curl --retry 5 http://$FISSION_ROUTER/$fn)
-
-log "Checking for valid response"
-echo $response | grep -i hello
-
 log "All done."