Add a feature to pass user provided files during qcow2ova image conversion

[Amulyam24]

What this PR does / why we need it:
This PR adds a feature to allow user to add custom files to the image built.
--write-files-list is a comma separated list of files and --write-to-dir-path is the path where the files are written in the built image

example:

pvsadm image qcow2ova --image-name test-image --image-dist centos --image-url https://cloud.centos.org/centos/10-stream/ppc64le/images/CentOS-Stream-GenericCloud-10-latest.ppc64le.qcow2 --prep-template ./image-prep.template --write-files-list key.pub,files --write-to-dir-path /home/user

Tested the following scenarios

1. Copy multiple files and directories
2. Copy nested directories
3. If destination path doesn't exist, creates and copies to it

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #811

Special notes for your reviewer:

**Output/Demonstration

Add a feature to pass user provided files during qcow2ova image conversion

[Prajyot-Parab]

/cc @kishen-v

[kishen-v]

/lgtm

[ppc64le-cloud-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Amulyam24
Once this PR has been reviewed and has the lgtm label, please assign mkumatag for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Amulyam24]

@kishen-v @anup-kodlekere, PTAL!

[anup-kodlekere]

LGTM!

[kishen-v]

/lgtm

[mkumatag]

Code Review

Thanks for the feature! I've left inline comments on the specific issues below. Summary of what needs attention before merge:

• 🔴 [High] Missing flag co-validation — --write-files-list without --write-to-dir-path silently dumps files into the image root
• 🟡 [Medium] coreos silently drops user-provided files with no warning
• 🟡 [Medium] MkdirAll runs unconditionally even when no files are specified
• 🟠 [Medium] No path traversal guard on --write-to-dir-path
• 🟡 [Low] os.Stat in copyFiles follows symlinks transparently
• 🟢 [Low] Double stat call in copyFile (minor)
• 🟡 [Medium] Broken example command (missing --image-name value)
• 🟡 [Medium] No unit tests for the new copy logic

Also worth considering: the flag names --write-files-list and --write-to-dir-path are more verbose than the project style. Something like --inject-files + --inject-dest would be terser and more intent-driven, matching how --prep-template, --cloud-config, --temp-dir are named.

Review done by Bob

[mkumatag]

[Correctness] Unconditional MkdirAll when no files are specified

When neither flag is provided, writeToDirPath is "" and writeFilesList is empty. filepath.Join(mnt, "") resolves to mnt (the image root), so MkdirAll runs on the root — harmless but unnecessary. More importantly, if a user passes --write-to-dir-path /some/path without --write-files-list, a spurious empty directory is silently created inside the image.

Wrap the entire block in a guard:

if len(writeFilesList) > 0 {
    imageWritePath := filepath.Join(mnt, writeToDirPath)
    if err := os.MkdirAll(imageWritePath, 0755); err != nil {
        return fmt.Errorf("failed to create directory %s: %w", imageWritePath, err)
    }
    for _, filePath := range writeFilesList {
        destinationPath := filepath.Join(imageWritePath, filepath.Base(filePath))
        if err := copyFiles(filePath, destinationPath); err != nil {
            return err
        }
    }
}

[mkumatag]

[Security] No path traversal guard on writeToDirPath

filepath.Join does not prevent ../ escape sequences. A user passing --write-to-dir-path ../../etc could resolve a path outside the intended mnt subtree.

Add a check after constructing imageWritePath:

if !strings.HasPrefix(filepath.Clean(imageWritePath), filepath.Clean(mnt)+"/") {
    return fmt.Errorf("write-to-dir-path %q escapes the image mount root", writeToDirPath)
}

[mkumatag]

[Correctness] coreos silently drops user-provided files

The file-copy block only runs inside prepare(), which is only called for rhel/centos. For coreos, any --write-files-list entries are silently ignored. A user would get no error and no warning.

Either propagate the copy step for coreos too, or return an explicit error when writeFilesList is non-empty for that distro:

case "coreos":
    if len(writeFilesList) > 0 {
        return fmt.Errorf("--write-files-list is not supported for coreos distro")
    }
    klog.Info("No image preparation required for the coreos.")
    return nil

[mkumatag]

[Security] os.Stat follows symlinks transparently

os.Stat resolves symlinks, so if any entry in --write-files-list is a symlink (e.g. pointing to /etc/shadow), it will be silently read and copied into the image as a regular file. Consider using os.Lstat here and explicitly deciding whether to follow, skip, or error on symlinks.

[mkumatag]

[Readability] Trailing blank line before closing brace

    return copyFile(src, dest)
                                // ← remove this blank line
}

gofmt does not produce a blank line before a closing brace. Minor, but inconsistent with the rest of the file.

[mkumatag]

[Performance] Double stat call in copyFile

os.Stat(src) was already called in copyFiles to determine if the path is a file or directory — then in.Stat() is called again here on the open file handle to retrieve permissions. You could pass fileInfo down from copyFiles to copyFile to avoid the redundant syscall:

func copyFile(src, dest string, srcInfo os.FileInfo) error {
    in, err := os.Open(src)
    ...
    out, err := os.OpenFile(dest, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, srcInfo.Mode())
    ...
}

[mkumatag]

[Correctness — High] Missing flag co-validation: --write-files-list requires --write-to-dir-path

When --write-files-list is provided without --write-to-dir-path, writeToDirPath defaults to "". filepath.Join(mnt, "") resolves to mnt (the image root), so all user files get dumped at / inside the image — almost certainly not intended.

Add a validation check in PreRunE, consistent with how other flag dependencies are validated in this file:

if len(opt.WriteFilesList) > 0 && opt.WriteToDirPath == "" {
    return fmt.Errorf("--write-to-dir-path is required when --write-files-list is specified")
}