Skip to content

SNOW-3202210: Implement snow validate-image CLI command#2809

Open
sfc-gh-ajiang wants to merge 7 commits intomainfrom
ajiang-SNOW_3202210-cre_validate
Open

SNOW-3202210: Implement snow validate-image CLI command#2809
sfc-gh-ajiang wants to merge 7 commits intomainfrom
ajiang-SNOW_3202210-cre_validate

Conversation

@sfc-gh-ajiang
Copy link

@sfc-gh-ajiang sfc-gh-ajiang commented Mar 10, 2026

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated automated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • I've confirmed that my changes are working by executing CLI's commands manually on MacOS.
  • I've confirmed that my changes are working by executing CLI's commands manually on Windows.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the release notes.
  • I've described my changes in the section below.
  • I've described my changes in the documentation.

Changes description

@sfc-gh-ajiang sfc-gh-ajiang requested a review from a team as a code owner March 10, 2026 18:05
@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch 2 times, most recently from fddd9a3 to b46515f Compare March 10, 2026 18:17
@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch from 32d5ae5 to 2f7d161 Compare March 10, 2026 18:59
Validates a Docker image against Snowflake custom image requirements.
"""
manager = CustomImageManager(config_path=DEFAULT_CONFIG_PATH)
report, output = manager.validate(image_hash=image_name)

Choose a reason for hiding this comment

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

nit: The image_name is passed as image_hash. This is a bit confusing because it’s not clear whether we expect an image name (e.g. repo:tag) or an image hash/ID. Could we standardize the terminology end-to-end?

@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch from c451fb6 to 63913e9 Compare March 12, 2026 17:30
"environment_variables": self._check_environment_variables,
"python_packages": self._check_python_packages,
"dependency_health": self._check_dependency_health,
"vulnerability_scan": self._check_vulnerabilities,

Choose a reason for hiding this comment

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

Why are we checking for vulnerabilities in the validate script? I thought we decided that the customer will own vuln scanning. If we provide this, it implies we are responsible for catching vulns?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching up this. I updated the vulnerability check to be the optional check. The default behavior is to skip this check

@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch from eb6685d to 57d0cb3 Compare March 12, 2026 20:35
Comment on lines +33 to +45
@app.command(requires_connection=False)
def validate(
image: str = typer.Argument(
...,
help="Local Docker image to validate. Accepts image name (e.g., 'myimage:latest') or image ID/hash.",
),
image_type: str = typer.Option(
"cpu",
"--image-type",
help="Base image type: 'cpu' or 'gpu'. Defaults to 'cpu'.",
),
**options,
) -> CommandResult:
Copy link
Contributor

Choose a reason for hiding this comment

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

The function signature accepts **options but never uses them, and critical options shown in the help text (--config and --show-all) are not defined. The test snapshots (test_help_messages.ambr lines 4720-4724) show these options exist in the help output, but they're missing from the actual implementation. This mismatch will confuse users who see the options in help but can't use them.

def validate(
    image: str = typer.Argument(...),
    image_type: str = typer.Option("cpu", "--image-type", ...),
    config: Optional[Path] = typer.Option(None, "--config", "-c", ...),
    show_all: bool = typer.Option(False, "--show-all", ...),
) -> CommandResult:

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Author

Choose a reason for hiding this comment

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

**options this is required in snowflake-cli framework

Copy link
Author

Choose a reason for hiding this comment

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

For the whitelisted packages, I hardcoded them here. This is a public repo. We cannot hardcoded the credentials here to access the snowflake-image-builder. This means we should update the whitelist manually. Any concerns or any suggestions @sfc-gh-wesong

Copy link
Author

Choose a reason for hiding this comment

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

we cannot detect the base image via docker primitives. So we do not have any checks on the base image. And for the image type, GPU or CPU, we rely on users' input. If they do not specify the image type in the command, we will assume that the image is a CPU image

@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch 2 times, most recently from b3ddfe5 to 3f91024 Compare March 17, 2026 04:17
@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch from 3f91024 to 3e8901b Compare March 17, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants