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

Verify if the submitted manifest YAML is consistent with the git repository #462

Conversation

iszulcdeepsense
Copy link
Collaborator

@iszulcdeepsense iszulcdeepsense commented May 16, 2024

Added

  • Submitted manifest YAML is compared with the one in the job's repository.
    In case of incosistencies, the deployment is aborted.
    This only happens if the optional setting verify_manifest_consistency is turned on in the Image Builder.

@anders314159
Copy link
Contributor

Looks good to me.
Here's what I tested:
VMC = verify_manifest_consistency: false/verify_manifest_consistency: true in the image_builder config.
local/git is --build-context=local/--build-context=git.
no complain = job is built and pushed. complain = it complains about diff in job.yaml vs job.yaml on git.

job.yaml VMC=False, local VMC=True, local VMC=False, git VMC=True, git
No diff no complain no complain no complain no complain
Diff no complain no complain no complain complain
piped input, no diff no complain no complain no complain no complain
piped input, diff no complain no complain no complain complain

Although I am wondering if VMC=True should check for changes even if --build-context=local, or maybe it should print an info line saying something like "VMC=True, but --build-context=local, so not checking changes against git" - but worded more smartly'er than that.

@iszulcdeepsense
Copy link
Collaborator Author

@anders314159

Although I am wondering if VMC=True should check for changes even if --build-context=local, or maybe it should print an info line saying something like "VMC=True, but --build-context=local, so not checking changes against git" - but worded more smartly'er than that.

Right, it makes sense not to check it for local build context, even though it should always pass having the same local file in the image builder.

@iszulcdeepsense iszulcdeepsense merged commit 933f242 into master May 22, 2024
2 checks passed
@iszulcdeepsense iszulcdeepsense deleted the 452-verify-if-the-submitted-manifest-yaml-is-consistent-with-the-git-repository branch May 22, 2024 09:33
@@ -65,7 +65,7 @@ def build_job_image(

workspace, repo_dir, git_version = prepare_workspace(workspaces_path, manifest, git_credentials, build_context, deployment_id)

if config.verify_manifest_consistency:
if config.verify_manifest_consistency and not build_context:
Copy link
Contributor

Choose a reason for hiding this comment

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

For the racetrack CLI:
Should it be and build_context==BuildContextMethod.git or something like that?
I think the above and not build_context is always false for the racetrack CLI, since it selects "BuildContextMethod.default" by default.
Not sure if the build_context is set, or not set, elsewhere in the project and might be None/False.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the image builder build_context parameter is something different. It's no longer an enum describing a type of the build context, but it's the actual compressed build context tarball, represented as bytes. It's not empty when build_context_method == BuildContextMethod.local, thus checking not build_context is enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool thanks

Choose a reason for hiding this comment

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

Le Dot.

Glad to see the progress here and the thought which have gone into it.

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.

Verify if the submitted manifest YAML is consistent with the git repository
3 participants