From a689aba5ef4cc4cbbd0c9b9a76be401dd140b72e Mon Sep 17 00:00:00 2001 From: Callahan Date: Tue, 16 Jul 2024 13:56:22 -0500 Subject: [PATCH 1/6] tests: disable `build-base: devel` spread tests (#4922) Disable `build-base: devel` spread tests until we have the resources to debug the underlying issue with 24.10 buildd images and `systemd-resolved` (#4921). We should be able to address this issue in 2024-Nov. Fixes #4910 (CRAFT-3105) Signed-off-by: Callahan Kovacs --- spread.yaml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spread.yaml b/spread.yaml index 9eef183a25..07feaa003a 100644 --- a/spread.yaml +++ b/spread.yaml @@ -321,10 +321,11 @@ suites: - ubuntu-20.04* - ubuntu-22.04* - tests/spread/core-devel/: - summary: tests of devel base snaps - environment: - SNAPCRAFT_BUILD_ENVIRONMENT: "" +# 'build-base: devel' fails due to an issue with the 24.10 buildd image (#4921) +# tests/spread/core-devel/: +# summary: tests of devel base snaps +# environment: +# SNAPCRAFT_BUILD_ENVIRONMENT: "" # General, core suite tests/spread/cross-compile/: From 157d0a182a433f45db379ea174130db98775e0b3 Mon Sep 17 00:00:00 2001 From: Callahan Date: Thu, 25 Jul 2024 08:18:05 -0500 Subject: [PATCH 2/6] tests: add spread tests for validation-sets (#4924) Adds a spread test for `edit-validation-sets` and `list-validation-sets`. Signed-off-by: Callahan Kovacs Co-authored-by: Alex Lowe --- .github/workflows/spread.yml | 4 +- spread.yaml | 9 ++++ .../store => store/basic-workflow}/task.yaml | 8 +--- tests/spread/store/validation-sets/editor.sh | 12 ++++++ tests/spread/store/validation-sets/task.yaml | 41 +++++++++++++++++++ 5 files changed, 66 insertions(+), 8 deletions(-) rename tests/spread/{general/store => store/basic-workflow}/task.yaml (91%) create mode 100755 tests/spread/store/validation-sets/editor.sh create mode 100644 tests/spread/store/validation-sets/task.yaml diff --git a/.github/workflows/spread.yml b/.github/workflows/spread.yml index 4bcfaa18e8..b6ae331d1b 100644 --- a/.github/workflows/spread.yml +++ b/.github/workflows/spread.yml @@ -110,10 +110,12 @@ jobs: name: Run spread env: SPREAD_GOOGLE_KEY: ${{ secrets.SPREAD_GOOGLE_KEY }} + SNAPCRAFT_ASSERTION_KEY: "${{ secrets.SNAPCRAFT_ASSERTION_KEY }}" SNAPCRAFT_STORE_CREDENTIALS_STAGING: "${{ secrets.SNAPCRAFT_STORE_CREDENTIALS_STAGING }}" SNAPCRAFT_STORE_CREDENTIALS_STAGING_CANDID: "${{ secrets.SNAPCRAFT_STORE_CREDENTIALS_STAGING_CANDID }}" SNAPCRAFT_STORE_CREDENTIALS_STAGING_LEGACY: "${{ secrets.SNAPCRAFT_STORE_CREDENTIALS_STAGING_LEGACY }}" - run: spread google:ubuntu-22.04-64:tests/spread/general/store + run: | + spread google:ubuntu-22.04-64:tests/spread/store/ - name: Discard spread workers if: always() diff --git a/spread.yaml b/spread.yaml index 07feaa003a..851683272d 100644 --- a/spread.yaml +++ b/spread.yaml @@ -377,6 +377,15 @@ suites: sudo apt-get install git sudo apt-mark auto git + tests/spread/store/: + summary: tests of store-related snapcraft commands + manual: true + environment: + STORE_DASHBOARD_URL: https://dashboard.staging.snapcraft.io + STORE_API_URL: https://api.staging.snapcraft.io + STORE_UPLOAD_URL: https://storage.staging.snapcraftcontent.com + UBUNTU_ONE_SSO_URL: https://login.staging.ubuntu.com + path: /snapcraft/ include: - tests/ diff --git a/tests/spread/general/store/task.yaml b/tests/spread/store/basic-workflow/task.yaml similarity index 91% rename from tests/spread/general/store/task.yaml rename to tests/spread/store/basic-workflow/task.yaml index 2a41cf3f47..c6035c2bcf 100644 --- a/tests/spread/general/store/task.yaml +++ b/tests/spread/store/basic-workflow/task.yaml @@ -1,6 +1,4 @@ -summary: Test the store workflow - -manual: true +summary: Test the store workflow to register, upload, and release a snap environment: # use a core22 snap with components but no component hooks @@ -9,10 +7,6 @@ environment: SNAPCRAFT_STORE_CREDENTIALS/ubuntu_one: "$(HOST: echo ${SNAPCRAFT_STORE_CREDENTIALS_STAGING})" SNAPCRAFT_STORE_CREDENTIALS/legacy: "$(HOST: echo ${SNAPCRAFT_STORE_CREDENTIALS_STAGING_LEGACY})" SNAPCRAFT_STORE_CREDENTIALS/candid: "$(HOST: echo ${SNAPCRAFT_STORE_CREDENTIALS_STAGING_CANDID})" - STORE_DASHBOARD_URL: https://dashboard.staging.snapcraft.io - STORE_API_URL: https://api.staging.snapcraft.io - STORE_UPLOAD_URL: https://storage.staging.snapcraftcontent.com - UBUNTU_ONE_SSO_URL: https://login.staging.ubuntu.com prepare: | if [[ -z "$SNAPCRAFT_STORE_CREDENTIALS" ]]; then diff --git a/tests/spread/store/validation-sets/editor.sh b/tests/spread/store/validation-sets/editor.sh new file mode 100755 index 0000000000..a38c96be47 --- /dev/null +++ b/tests/spread/store/validation-sets/editor.sh @@ -0,0 +1,12 @@ +#! /bin/bash + +validation_set_file="$1" + +# flip-flop between two valid revisions of `test-snapcraft-assertions` in the staging store: 1 and 2 +if grep -q "^ revision:.*1" "$validation_set_file"; then + (( revision=2 )) +else + (( revision=1 )) +fi + +sed -i "s/ revision:.*/ revision: $revision/g" "$validation_set_file" diff --git a/tests/spread/store/validation-sets/task.yaml b/tests/spread/store/validation-sets/task.yaml new file mode 100644 index 0000000000..277ee9d8e1 --- /dev/null +++ b/tests/spread/store/validation-sets/task.yaml @@ -0,0 +1,41 @@ +summary: test the validation-set commands + +environment: + SNAPCRAFT_ASSERTION_KEY: "$(HOST: echo ${SNAPCRAFT_ASSERTION_KEY})" + SNAPCRAFT_STORE_CREDENTIALS: "$(HOST: echo ${SNAPCRAFT_STORE_CREDENTIALS_STAGING})" + +prepare: | + if [[ -z "$SNAPCRAFT_STORE_CREDENTIALS" ]]; then + ERROR "No credentials set in env SNAPCRAFT_STORE_CREDENTIALS" + fi + + if [[ -z "$SNAPCRAFT_ASSERTION_KEY" ]]; then + ERROR "No gpg key set in env SNAPCRAFT_ASSERTION_KEY" + fi + + # setup snap gpg dir + mkdir -p "$HOME/.snap/gnupg" + chmod 700 "$HOME/.snap/gnupg" + + # import a registered key + echo "$SNAPCRAFT_ASSERTION_KEY" | base64 --decode > store-key.txt + gpg --homedir "$HOME/.snap/gnupg" --import store-key.txt + rm -f store-key.txt + + snap install yq + +execute: | + # ensure snapcraft is logged in and can access the store + snapcraft whoami + + # snapcraft will use a fake file editor + export EDITOR="$PWD/editor.sh" + + snapcraft edit-validation-sets "$(snapcraft whoami | yq .id)" testset 1 --key-name testspreadkey + + snapcraft list-validation-sets | MATCH testset + +restore: | + rm -rf "$HOME/.snap/gnupg" + + snap remove --purge yq From cae7ffbcfd480057f595c6aa8cd31ab42ff3319b Mon Sep 17 00:00:00 2001 From: Callahan Date: Tue, 30 Jul 2024 12:36:22 -0500 Subject: [PATCH 3/6] fix: use correct types for validation set models (#4928) - Use the correct data types for API calls - Switch to pydantic models - Catch model errors locally rather than relying on the store to validate the data - Make the boilerplate validation set valid Signed-off-by: Callahan Kovacs --- snapcraft/commands/validation_sets.py | 62 ++-- snapcraft_legacy/cli/assertions.py | 3 +- snapcraft_legacy/storeapi/v2/_api_schema.py | 74 +---- .../storeapi/v2/validation_sets.py | 280 ++++++++---------- tests/legacy/unit/store/test_store_client.py | 4 +- .../unit/store/v2/test_validation_sets.py | 215 +++++++------- tests/unit/commands/test_validation_sets.py | 189 +++++++----- 7 files changed, 404 insertions(+), 423 deletions(-) diff --git a/snapcraft/commands/validation_sets.py b/snapcraft/commands/validation_sets.py index 2f01221dc0..50f1ef97ed 100644 --- a/snapcraft/commands/validation_sets.py +++ b/snapcraft/commands/validation_sets.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2022 Canonical Ltd. +# Copyright 2022,2024 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -24,14 +24,17 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, Dict, Optional +import craft_application.util import yaml from craft_application.commands import AppCommand +from craft_application.errors import CraftValidationError from craft_cli import emit from overrides import overrides from snapcraft import errors, utils from snapcraft_legacy._store import StoreClientCLI from snapcraft_legacy.storeapi.errors import StoreValidationSetsError +from snapcraft_legacy.storeapi.v2 import validation_sets if TYPE_CHECKING: import argparse @@ -40,7 +43,7 @@ _VALIDATIONS_SETS_SNAPS_TEMPLATE = textwrap.dedent( """\ snaps: - # - name: # The name of the snap. + - name: hello # The name of the snap. # id: # The ID of the snap. Optional, defaults to the current ID for # the provided name. # presence: [required|optional|invalid] # Optional, defaults to required. @@ -102,7 +105,9 @@ def run(self, parsed_args: "argparse.Namespace"): validation_sets_path.write_text(validation_sets_template, encoding="utf-8") edited_validation_sets = edit_validation_sets(validation_sets_path) - if edited_validation_sets == yaml.safe_load(validation_sets_template): + if edited_validation_sets == validation_sets.EditableBuildAssertion( + **yaml.safe_load(validation_sets_template) + ): emit.message("No changes made") return @@ -119,7 +124,7 @@ def run(self, parsed_args: "argparse.Namespace"): "Do you wish to amend the validation set?" ): raise errors.SnapcraftError( - "Operation aborted" + "operation aborted" ) from validation_error edited_validation_sets = edit_validation_sets(validation_sets_path) finally: @@ -127,32 +132,43 @@ def run(self, parsed_args: "argparse.Namespace"): def _submit_validation_set( - edited_validation_sets: Dict[str, Any], + edited_validation_sets: validation_sets.EditableBuildAssertion, key_name: Optional[str], store_client: StoreClientCLI, ) -> None: + emit.debug(f"Posting assertion to build: {edited_validation_sets.json()}") build_assertion = store_client.post_validation_sets_build_assertion( - validation_sets=edited_validation_sets + validation_sets=edited_validation_sets.marshal() ) - signed_validation_sets = _sign_assertion( - build_assertion.marshal(), key_name=key_name + build_assertion_dict = build_assertion.marshal_scalars_as_strings() + + signed_validation_sets = _sign_assertion(build_assertion_dict, key_name=key_name) + + emit.debug("Posting signed validation sets.") + response = store_client.post_validation_sets( + signed_validation_sets=signed_validation_sets ) - store_client.post_validation_sets(signed_validation_sets=signed_validation_sets) + emit.debug(f"Response: {response.json()}") def _generate_template( - asserted_validation_sets, *, account_id: str, set_name: str, sequence: str + asserted_validation_sets: validation_sets.ValidationSets, + *, + account_id: str, + set_name: str, + sequence: str, ) -> str: """Generate a template to edit asserted_validation_sets.""" try: # assertions should only have one item since a specific # sequence was requested. - revision = asserted_validation_sets.assertions[0].revision + revision = asserted_validation_sets.assertions[0].headers.revision snaps = yaml.dump( { "snaps": [ - s.marshal() for s in asserted_validation_sets.assertions[0].snaps + s.marshal() + for s in asserted_validation_sets.assertions[0].headers.snaps ] }, default_flow_style=False, @@ -174,7 +190,9 @@ def _generate_template( return unverified_validation_sets -def edit_validation_sets(validation_sets_path: Path) -> Dict[str, Any]: +def edit_validation_sets( + validation_sets_path: Path, +) -> validation_sets.EditableBuildAssertion: """Spawn an editor to modify the validation-sets.""" editor_cmd = os.getenv("EDITOR", "vi") @@ -182,23 +200,29 @@ def edit_validation_sets(validation_sets_path: Path) -> Dict[str, Any]: with emit.pause(): subprocess.run([editor_cmd, validation_sets_path], check=True) try: - edited_validation_sets = yaml.safe_load( - validation_sets_path.read_text(encoding="utf-8") + with validation_sets_path.open() as file: + data = craft_application.util.safe_yaml_load(file) + edited_validation_sets = validation_sets.EditableBuildAssertion.from_yaml_data( + data=data, + # filepath is only shown for pydantic errors and snapcraft should + # not expose the temp file name + filepath=Path("validation-sets"), ) return edited_validation_sets - except yaml.YAMLError as yaml_error: - emit.message(f"A YAML parsing error occurred {yaml_error!s}") + except (yaml.YAMLError, CraftValidationError) as err: + emit.message(f"{err!s}") if not utils.confirm_with_user("Do you wish to amend the validation set?"): - raise errors.SnapcraftError("Operation aborted") from yaml_error + raise errors.SnapcraftError("operation aborted") from err def _sign_assertion(assertion: Dict[str, Any], *, key_name: Optional[str]) -> bytes: + emit.debug("Signing assertion.") cmdline = ["snap", "sign"] if key_name: cmdline += ["-k", key_name] snap_sign = subprocess.Popen(cmdline, stdin=subprocess.PIPE, stdout=subprocess.PIPE) signed_assertion, _ = snap_sign.communicate(input=json.dumps(assertion).encode()) if snap_sign.returncode != 0: - raise errors.SnapcraftError("Failed to sign assertion") + raise errors.SnapcraftError("failed to sign assertion") return signed_assertion diff --git a/snapcraft_legacy/cli/assertions.py b/snapcraft_legacy/cli/assertions.py index c4b73937cb..8177264ed8 100644 --- a/snapcraft_legacy/cli/assertions.py +++ b/snapcraft_legacy/cli/assertions.py @@ -135,7 +135,8 @@ def list_validation_sets(name, sequence): else: headers = ["Account-ID", "Name", "Sequence", "Revision", "When"] assertions = list() - for assertion in asserted_validation_sets.assertions: + for assertion_header in asserted_validation_sets.assertions: + assertion = assertion_header.headers assertions.append( [ assertion.account_id, diff --git a/snapcraft_legacy/storeapi/v2/_api_schema.py b/snapcraft_legacy/storeapi/v2/_api_schema.py index 8e56fca0e4..1bde67d5e9 100644 --- a/snapcraft_legacy/storeapi/v2/_api_schema.py +++ b/snapcraft_legacy/storeapi/v2/_api_schema.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2020 Canonical Ltd +# Copyright 2020,2024 Canonical Ltd # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -275,75 +275,3 @@ "required": ["account", "channels", "packages", "permissions"], "type": "object", } - -# https://dashboard.snapcraft.io/docs/v2/en/validation-sets.html -BUILD_ASSERTION_JSONSCHEMA: Dict[str, Any] = { - "properties": { - "account-id": { - "description": 'The "account-id" assertion header', - "type": "string", - }, - "authority-id": { - "description": 'The "authority-id" assertion header', - "type": "string", - }, - "name": {"description": 'The "name" assertion header', "type": "string"}, - "revision": { - "description": 'The "revision" assertion header', - "type": "string", - }, - "sequence": { - "description": 'The "sequence" assertion header', - "type": "string", - }, - "series": {"description": 'The "series" assertion header', "type": "string"}, - "snaps": { - "items": { - "description": "List of snaps in a Validation Set assertion", - "properties": { - "id": { - "description": "Snap ID", - "maxLength": 100, - "type": "string", - }, - "name": { - "description": "Snap name", - "maxLength": 100, - "type": "string", - }, - "presence": { - "description": "Snap presence", - "enum": ["required", "optional", "invalid"], - "type": "string", - }, - "revision": {"description": "Snap revision", "type": "string"}, - }, - "required": ["name"], - "type": "object", - }, - "minItems": 1, - "type": "array", - }, - "timestamp": { - "description": 'The "timestamp" assertion header', - "type": "string", - }, - "type": { - "const": "validation-set", - "description": 'The "type" assertion header', - "type": "string", - }, - }, - "required": [ - "type", - "authority-id", - "series", - "account-id", - "name", - "sequence", - # "revision", - "timestamp", - "snaps", - ], - "type": "object", -} diff --git a/snapcraft_legacy/storeapi/v2/validation_sets.py b/snapcraft_legacy/storeapi/v2/validation_sets.py index ad4c5e0ca8..3dbe84f6b9 100644 --- a/snapcraft_legacy/storeapi/v2/validation_sets.py +++ b/snapcraft_legacy/storeapi/v2/validation_sets.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright (C) 2021 Canonical Ltd +# Copyright (C) 2021,2024 Canonical Ltd # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -14,161 +14,135 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from typing import Any, Dict, List, Optional +from typing import Any, Annotated, Dict, Literal, TYPE_CHECKING +import numbers +import pydantic -import jsonschema +from craft_application import models -from ._api_schema import BUILD_ASSERTION_JSONSCHEMA +if TYPE_CHECKING: + SnapName = str + SnapId = str +else: + SnapName = pydantic.constr(max_length=40) + SnapId = pydantic.constr(max_length=40) -class Snap: + +def cast_dict_scalars_to_strings(data: dict) -> dict[str, Any]: + """Cast all scalars in a dictionary to strings. + + Supported scalar types are str, bool, and numbers. + """ + return {_to_string(key): _to_string(value) for key, value in data.items()} + + +def _to_string( + data: dict | list | str | int | float | str | bool | None +) -> dict[str, Any] | list | str | None: + """Recurse through nested dicts and lists and cast scalar values to strings. + + Supported scalar types are str, bool, and numbers. + """ + # start with a string as it is the most common scenario + if isinstance(data, str): + return data + + if isinstance(data, dict): + return {_to_string(key): _to_string(value) for key, value in data.items()} + + if isinstance(data, list): + return [_to_string(i) for i in data] + + if isinstance(data, (numbers.Number, bool)): + return str(data) + + return data + + +class Snap(models.CraftBaseModel): """Represent a Snap in a Validation Set.""" - @classmethod - def unmarshal(cls, payload: Dict[str, Any]) -> "Snap": - jsonschema.validate( - payload, BUILD_ASSERTION_JSONSCHEMA["properties"]["snaps"]["items"] - ) - - return cls( - name=payload["name"], - snap_id=payload.get("id"), - presence=payload.get("presence"), - revision=payload.get("revision"), - ) - - def marshal(self) -> Dict[str, Any]: - payload = {"name": self.name} - - if self.snap_id is not None: - payload["id"] = self.snap_id - - if self.presence is not None: - payload["presence"] = self.presence - - if self.revision is not None: - payload["revision"] = self.revision - - return payload - - def __eq__(self, other) -> bool: - if not isinstance(other, Snap): - return False - - return ( - self.name == other.name - and self.snap_id == other.snap_id - and self.presence == other.presence - and self.revision == other.revision - ) - - def __repr__(self) -> str: - return f"<{self.__class__.__name__}: {self.name!r}>" - - def __init__( - self, - name: str, - snap_id: Optional[str] = None, - presence: Optional[str] = None, - revision: Optional[str] = None, - ): - self.name = name - self.snap_id = snap_id - self.presence = presence - self.revision = revision - - -class BuildAssertion: - """Represent Validation Set assertion headers ready local signing.""" - - @classmethod - def unmarshal(cls, payload: Dict[str, Any]) -> "BuildAssertion": - jsonschema.validate(payload, BUILD_ASSERTION_JSONSCHEMA) - - return cls( - account_id=payload["account-id"], - authority_id=payload["authority-id"], - name=payload["name"], - revision=payload.get("revision", "0"), - sequence=payload["sequence"], - series=payload["series"], - snaps=[Snap.unmarshal(s) for s in payload["snaps"]], - timestamp=payload["timestamp"], - assertion_type=payload["type"], - ) - - def marshal(self) -> Dict[str, Any]: - return { - "account-id": self.account_id, - "authority-id": self.authority_id, - "name": self.name, - "revision": self.revision, - "sequence": self.sequence, - "snaps": [s.marshal() for s in self.snaps], - "timestamp": self.timestamp, - "type": self.assertion_type, - "series": self.series, - } - - def __eq__(self, other) -> bool: - if not isinstance(other, BuildAssertion): - return False - - return ( - self.name == other.name - and self.account_id == other.account_id - and self.authority_id == other.authority_id - and self.revision == other.revision - and self.sequence == other.sequence - and self.snaps == other.snaps - and self.timestamp == other.timestamp - and self.assertion_type == other.assertion_type - and self.series == other.series - ) - - def __repr__(self) -> str: - return f"<{self.__class__.__name__}: {self.name!r}>" - - def __init__( - self, - *, - account_id: str, - authority_id: str, - name: str, - revision: str, - sequence: str, - snaps: List[Snap], - timestamp: str, - assertion_type: str, - series: str = "16", - ): - self.account_id = account_id - self.authority_id = authority_id - self.name = name - self.revision = revision - self.sequence = sequence - self.snaps = snaps - self.series = series - self.timestamp = timestamp - self.assertion_type = assertion_type - - -class ValidationSets: - @classmethod - def unmarshal(cls, payload: Dict[str, Any]) -> "ValidationSets": - return cls( - assertions=[ - BuildAssertion.unmarshal(a["headers"]) - for a in payload.get("assertions", list()) - ] - ) - - def marshal(self) -> Dict[str, Any]: - return {"assertions": [{"headers": a.marshal()} for a in self.assertions]} - - def __repr__(self) -> str: - assertion_count = len(self.assertions) - return f"<{self.__class__.__name__}: assertions {assertion_count!r}>" - - def __init__(self, assertions: List[BuildAssertion]) -> None: - self.assertions = assertions + name: SnapName + """Snap name""" + + id: SnapId | None + """Snap ID""" + + presence: Literal["required", "optional", "invalid"] | None + """Snap presence""" + + revision: int | None + """Snap revision""" + + +class EditableBuildAssertion(models.CraftBaseModel): + """Subset of a build assertion that can be edited by the user. + + https://dashboard.snapcraft.io/docs/reference/v2/en/validation-sets.html#request-json-schema + """ + account_id: str + """The "account-id" assertion header""" + + name: str + """The "name" assertion header""" + + revision: str | None + """The "revision" assertion header""" + + sequence: int + """The "sequence" assertion header""" + + snaps: Annotated[list[Snap], pydantic.Field(min_items=1)] + """List of snaps in a Validation Set assertion""" + + def marshal_scalars_as_strings(self) -> Dict[str, Any]: + """Marshal the object where all scalars are represented as strings.""" + data = self.marshal() + return cast_dict_scalars_to_strings(data) + + +class BuildAssertion(EditableBuildAssertion): + """Full build assertion header for a Validation Set. + + https://dashboard.snapcraft.io/docs/reference/v2/en/validation-sets.html#response-json-schema + """ + authority_id: str + """The "authority-id" assertion header""" + + series: str + """The "series" assertion header""" + + sign_key_sha3_384: str | None + """Signing key ID.""" + + timestamp: str + """The "timestamp" assertion header""" + + type: Literal["validation-set"] + """The "type" assertion header""" + + @pydantic.root_validator(pre=True) + def remove_sign_key(cls, values): + """Accept but always ignore the sign key. + + The API can return and accept the sign key but the previous implementation + ignored it. + """ + values.pop("sign-key-sha3-384", None) + return values + + +class Headers(models.CraftBaseModel): + """Assertion headers for a validation set.""" + headers: BuildAssertion + """Assertion headers""" + + +class ValidationSets(models.CraftBaseModel): + """Validation sets. + + https://dashboard.snapcraft.io/docs/reference/v2/en/validation-sets.html#id4 + """ + assertions: list[Headers] + """List of validation-set assertions""" diff --git a/tests/legacy/unit/store/test_store_client.py b/tests/legacy/unit/store/test_store_client.py index ef89aa0eb1..3309bf13bf 100644 --- a/tests/legacy/unit/store/test_store_client.py +++ b/tests/legacy/unit/store/test_store_client.py @@ -438,7 +438,7 @@ def test_post_valid_assertion(self): self.assertThat(vs, IsInstance(validation_sets.ValidationSets)) self.assertThat(vs.assertions, HasLength(1)) - self.assertThat(vs.assertions[0], Equals(build_assertion)) + self.assertThat(vs.assertions[0].headers, Equals(build_assertion)) def test_post_invalid_assertion(self): build_assertion = self.client.post_validation_sets_build_assertion( @@ -516,7 +516,7 @@ def test_get_validation_sets_by_name(self): self.assertThat(vs, IsInstance(validation_sets.ValidationSets)) self.expectThat(vs.assertions, HasLength(1)) - self.expectThat(vs.assertions[0].name, Equals("acme-cert-2020-10")) + self.expectThat(vs.assertions[0].headers.name, Equals("acme-cert-2020-10")) def test_get_invalid_sequence(self): raised = self.assertRaises( diff --git a/tests/legacy/unit/store/v2/test_validation_sets.py b/tests/legacy/unit/store/v2/test_validation_sets.py index e63db9188f..2820efab48 100644 --- a/tests/legacy/unit/store/v2/test_validation_sets.py +++ b/tests/legacy/unit/store/v2/test_validation_sets.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright (C) 2021 Canonical Ltd +# Copyright (C) 2021,2024 Canonical Ltd # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -19,127 +19,136 @@ from snapcraft_legacy.storeapi.v2 import validation_sets -@pytest.mark.parametrize("snap_id", (None, "snap_id")) -@pytest.mark.parametrize("presence", (None, "required", "optional", "invalid")) -@pytest.mark.parametrize("revision", ("42", None)) -def test_snap(snap_id, presence, revision): - payload = { - "name": "set-1", +@pytest.fixture() +def fake_snap_data(): + return { + "name": "snap-name", + "id": "snap-id", + "presence": "required", + "revision": 42, } - if presence is not None: - payload["presence"] = presence - if snap_id is not None: - payload["id"] = snap_id +@pytest.fixture() +def fake_snap(fake_snap_data): + return validation_sets.Snap.unmarshal(fake_snap_data) - if revision is not None: - payload["revision"] = revision - s = validation_sets.Snap.unmarshal(payload) +@pytest.fixture() +def fake_editable_build_assertion_data(fake_snap_data): + return { + "account-id": "account-id-1", + "name": "validation-set-1", + "revision": "revision-1", + "sequence": 1, + "snaps": [fake_snap_data], + } + - assert repr(s) == "" - assert s.name == payload["name"] - assert s.snap_id == payload.get("id") - assert s.presence == payload.get("presence") - assert s.revision == payload.get("revision") - assert s.marshal() == payload +@pytest.fixture() +def fake_editable_build_assertion(fake_editable_build_assertion_data): + return validation_sets.EditableBuildAssertion.unmarshal( + fake_editable_build_assertion_data + ) -@pytest.mark.parametrize("revision", ("42", None)) -def test_build_assertion(revision): - payload = { +@pytest.fixture() +def fake_build_assertion_data(fake_snap_data): + return { "account-id": "account-id-1", - "authority-id": "authority-id-1", "name": "validation-set-1", - "sequence": "1", + "revision": "revision-1", + "sequence": 1, + "snaps": [fake_snap_data], + "authority-id": "authority-id-1", "series": "16", - "snaps": [validation_sets.Snap(name="snap-name").marshal()], + "sign-key-sha3-384": "sign-key-1", "timestamp": "2020-10-29T16:36:56Z", "type": "validation-set", } - if revision is not None: - payload["revision"] = revision - s = validation_sets.BuildAssertion.unmarshal(payload) - - assert repr(s) == "" - assert s.account_id == payload["account-id"] - assert s.authority_id == payload["authority-id"] - assert s.name == payload["name"] - assert s.sequence == payload.get("sequence") - assert s.series == payload.get("series") - assert s.snaps == [validation_sets.Snap(name="snap-name")] - assert s.timestamp == "2020-10-29T16:36:56Z" - - if revision is None: - payload["revision"] = "0" - - assert s.marshal() == payload - - -def test_validation_sets(): - payload = { - "assertions": [ - { - "headers": { - "account-id": "account-id-1", - "authority-id": "authority-id-1", - "name": "validation-set-1", - "revision": "1", - "sequence": "1", - "series": "16", - "snaps": [validation_sets.Snap(name="snap-name-1").marshal()], - "timestamp": "2020-10-29T16:36:56Z", - "type": "validation-set", - } - }, +@pytest.fixture() +def fake_build_assertion(fake_build_assertion_data): + return validation_sets.BuildAssertion.unmarshal(fake_build_assertion_data) + + +@pytest.mark.parametrize( + ("input_dict", "expected"), + [ + pytest.param({}, {}, id="empty"), + pytest.param( + {False: False, True: True}, + {"False": "False", "True": "True"}, + id="boolean values", + ), + pytest.param( + {0: 0, None: None, "dict": {}, "list": [], "str": ""}, + ({"0": "0", None: None, "dict": {}, "list": [], "str": ""}), + id="none-like values", + ), + pytest.param( + {10: 10, 20.0: 20.0, "30": "30", True: True}, + {"10": "10", "20.0": "20.0", "30": "30", "True": "True"}, + id="scalar values", + ), + pytest.param( + {"foo": {"bar": [1, 2.0], "baz": {"qux": True}}}, + {"foo": {"bar": ["1", "2.0"], "baz": {"qux": "True"}}}, + id="nested data structures", + ), + ], +) +def test_cast_dict_scalars_to_strings(input_dict, expected): + actual = validation_sets.cast_dict_scalars_to_strings(input_dict) + + assert actual == expected + + +def test_ignore_sign_key(fake_build_assertion): + """Ignore the sign key when unmarshalling.""" + assert not fake_build_assertion.sign_key_sha3_384 + + +def test_editable_build_assertion_marshal_as_str(fake_editable_build_assertion): + """Cast all scalars to string when marshalling.""" + data = fake_editable_build_assertion.marshal_scalars_as_strings() + + assert data == { + "account-id": "account-id-1", + "name": "validation-set-1", + "revision": "revision-1", + "sequence": "1", + "snaps": [ { - "headers": { - "account-id": "account-id-1", - "authority-id": "authority-id-1", - "name": "validation-set-1", - "revision": "3", - "sequence": "1", - "series": "16", - "snaps": [validation_sets.Snap(name="snap-name-2").marshal()], - "timestamp": "2020-10-29T16:36:56Z", - "type": "validation-set", - }, + "id": "snap-id", + "name": "snap-name", + "presence": "required", + "revision": "42", }, - ] + ], } - s = validation_sets.ValidationSets.unmarshal(payload) - - assert repr(s) == "" - assert s.assertions[0] == validation_sets.BuildAssertion.unmarshal( - { - "account-id": "account-id-1", - "authority-id": "authority-id-1", - "name": "validation-set-1", - "revision": "1", - "sequence": "1", - "series": "16", - "snaps": [validation_sets.Snap(name="snap-name-1").marshal()], - "timestamp": "2020-10-29T16:36:56Z", - "type": "validation-set", - }, - ) - assert s.assertions[1] == validation_sets.BuildAssertion.unmarshal( - { - "account-id": "account-id-1", - "authority-id": "authority-id-1", - "name": "validation-set-1", - "revision": "3", - "sequence": "1", - "series": "16", - "snaps": [validation_sets.Snap(name="snap-name-2").marshal()], - "timestamp": "2020-10-29T16:36:56Z", - "type": "validation-set", - } - ) +def test_build_assertion_marshal_as_str(fake_build_assertion): + """Cast all scalars to string when marshalling.""" + data = fake_build_assertion.marshal_scalars_as_strings() - assert s.marshal() == payload + assert data == { + "account-id": "account-id-1", + "authority-id": "authority-id-1", + "name": "validation-set-1", + "revision": "revision-1", + "sequence": "1", + "series": "16", + "snaps": [ + { + "id": "snap-id", + "name": "snap-name", + "presence": "required", + "revision": "42", + }, + ], + "timestamp": "2020-10-29T16:36:56Z", + "type": "validation-set", + } diff --git a/tests/unit/commands/test_validation_sets.py b/tests/unit/commands/test_validation_sets.py index f840a27c9f..1e69a278d1 100644 --- a/tests/unit/commands/test_validation_sets.py +++ b/tests/unit/commands/test_validation_sets.py @@ -32,63 +32,71 @@ # Fixtures # ############ +VALIDATION_SET_DATA = { + "assertions": [ + { + "headers": { + "account-id": "AccountIDXXXOfTheRequestingUserX", + "authority-id": "AccountIDXXXOfTheRequestingUserX", + "name": "certification-x1", + "revision": "222", + "sequence": "9", + "series": "16", + "sign-key-sha3-384": "XSignXKeyXHashXXXXXXXXXXX", + "snaps": [ + { + "id": "XXSnapIDForXSnapName1XXXXXXXXXXX", + "name": "snap-name-1", + "presence": "optional", + "revision": "10", + }, + { + "id": "XXSnapIDForXSnapName2XXXXXXXXXXX", + "name": "snap-name-2", + }, + ], + "timestamp": "2020-10-29T16:36:56Z", + "type": "validation-set", + } + }, + ] +} + @pytest.fixture -def validation_sets_payload(): - return validation_sets.ValidationSets.unmarshal( - { - "assertions": [ - { - "headers": { - "account-id": "AccountIDXXXOfTheRequestingUserX", - "authority-id": "AccountIDXXXOfTheRequestingUserX", - "name": "certification-x1", - "revision": "222", - "sequence": "9", - "series": "16", - "sign-key-sha3-384": "XSignXKeyXHashXXXXXXXXXXX", - "snaps": [ - { - "id": "XXSnapIDForXSnapName1XXXXXXXXXXX", - "name": "snap-name-1", - "presence": "optional", - }, - { - "id": "XXSnapIDForXSnapName2XXXXXXXXXXX", - "name": "snap-name-2", - }, - ], - "timestamp": "2020-10-29T16:36:56Z", - "type": "validation-set", - } - }, - ] - } +def fake_validation_sets(): + return validation_sets.ValidationSets.unmarshal(VALIDATION_SET_DATA) + + +@pytest.fixture() +def fake_build_assertion(): + return validation_sets.BuildAssertion.unmarshal( + VALIDATION_SET_DATA["assertions"][0]["headers"] ) @pytest.fixture -def fake_dashboard_post_validation_sets(validation_sets_payload, mocker): +def fake_dashboard_post_validation_sets(fake_validation_sets, mocker): return mocker.patch.object( - StoreClientCLI, "post_validation_sets", return_value=validation_sets_payload + StoreClientCLI, "post_validation_sets", return_value=fake_validation_sets ) @pytest.fixture -def fake_dashboard_post_validation_sets_build_assertion( - validation_sets_payload, mocker -): +def fake_dashboard_post_validation_sets_build_assertion(fake_validation_sets, mocker): return mocker.patch.object( StoreClientCLI, "post_validation_sets_build_assertion", - return_value=validation_sets_payload.assertions[0], + return_value=validation_sets.BuildAssertion.unmarshal( + VALIDATION_SET_DATA["assertions"][0]["headers"] + ), ) @pytest.fixture -def fake_dashboard_get_validation_sets(validation_sets_payload, mocker): +def fake_dashboard_get_validation_sets(fake_validation_sets, mocker): return mocker.patch.object( - StoreClientCLI, "get_validation_sets", return_value=validation_sets_payload + StoreClientCLI, "get_validation_sets", return_value=fake_validation_sets ) @@ -104,19 +112,22 @@ def sign(assertion: Dict[str, Any], *, key_name: str) -> bytes: @pytest.fixture def edit_return_value(): - return { - "account-id": "AccountIDXXXOfTheRequestingUserX", - "name": "certification-x1", - "sequence": "9", - "snaps": [ - { - "id": "XXSnapIDForXSnapName1XXXXXXXXXXX", - "name": "snap-name-1", - "presence": "required", - }, - {"id": "XXSnapIDForXSnapName2XXXXXXXXXXX", "name": "snap-name-2"}, - ], - } + return validation_sets.EditableBuildAssertion.unmarshal( + { + "account-id": "AccountIDXXXOfTheRequestingUserX", + "name": "certification-x1", + "sequence": "9", + "snaps": [ + { + "id": "XXSnapIDForXSnapName1XXXXXXXXXXX", + "name": "snap-name-1", + "presence": "required", + "revision": "10", + }, + {"id": "XXSnapIDForXSnapName2XXXXXXXXXXX", "name": "snap-name-2"}, + ], + } + ) @pytest.fixture @@ -137,12 +148,13 @@ def fake_edit_validation_sets(mocker, edit_return_value): validation_sets={ "account-id": "AccountIDXXXOfTheRequestingUserX", "name": "certification-x1", - "sequence": "9", + "sequence": 9, "snaps": [ { "name": "snap-name-1", "id": "XXSnapIDForXSnapName1XXXXXXXXXXX", "presence": "required", + "revision": 10, }, {"name": "snap-name-2", "id": "XXSnapIDForXSnapName2XXXXXXXXXXX"}, ], @@ -150,13 +162,12 @@ def fake_edit_validation_sets(mocker, edit_return_value): ) EXPECTED_SIGNED_VALIDATION_SETS = ( - '{{"account-id": "AccountIDXXXOfTheRequestingUserX", "authority-id": ' - '"AccountIDXXXOfTheRequestingUserX", "name": "certification-x1", ' - '"revision": "222", "sequence": "9", "snaps": [{{"name": "snap-name-1", ' - '"id": "XXSnapIDForXSnapName1XXXXXXXXXXX", "presence": "optional"}}, ' + '{{"account-id": "AccountIDXXXOfTheRequestingUserX", "name": "certification-x1", ' + '"revision": "222", "sequence": "9", "snaps": [{{"name": "snap-name-1", "id": ' + '"XXSnapIDForXSnapName1XXXXXXXXXXX", "presence": "optional", "revision": "10"}}, ' '{{"name": "snap-name-2", "id": "XXSnapIDForXSnapName2XXXXXXXXXXX"}}], ' - '"timestamp": "2020-10-29T16:36:56Z", "type": "validation-set", ' - '"series": "16"}}\n\nSIGNED{key_name}' + '"authority-id": "AccountIDXXXOfTheRequestingUserX", "series": "16", "timestamp": ' + '"2020-10-29T16:36:56Z", "type": "validation-set"}}\n\nSIGNED{key_name}' ) @@ -176,8 +187,8 @@ def test_edit_validation_sets_with_no_changes_to_existing_set( emitter, ): # Make it look like there were no edits. - edit_return_value["sequence"] = 9 - edit_return_value["snaps"][0]["presence"] = "optional" + edit_return_value.sequence = 9 + edit_return_value.snaps[0].presence = "optional" fake_edit_validation_sets.return_value = edit_return_value cmd = commands.StoreEditValidationSetsCommand(None) @@ -203,7 +214,8 @@ def test_edit_validation_sets_with_no_changes_to_existing_set( @pytest.mark.usefixtures("memory_keyring", "fake_edit_validation_sets") @pytest.mark.parametrize("key_name", [None, "general", "main"]) def test_edit_validation_sets_with_changes_to_existing_set( - validation_sets_payload, + fake_validation_sets, + fake_build_assertion, fake_dashboard_get_validation_sets, fake_dashboard_post_validation_sets_build_assertion, fake_dashboard_post_validation_sets, @@ -232,11 +244,11 @@ def test_edit_validation_sets_with_changes_to_existing_set( signed_validation_sets=EXPECTED_SIGNED_VALIDATION_SETS.format( key_name=key_name ).encode() - ) + ), ] assert fake_snap_sign.mock_calls == [ call( - validation_sets_payload.assertions[0].marshal(), + fake_build_assertion.marshal_scalars_as_strings(), key_name=key_name, ) ] @@ -244,7 +256,8 @@ def test_edit_validation_sets_with_changes_to_existing_set( @pytest.mark.usefixtures("memory_keyring", "fake_edit_validation_sets") def test_edit_validation_sets_with_errors_to_amend( - validation_sets_payload, + fake_validation_sets, + fake_build_assertion, fake_dashboard_get_validation_sets, fake_dashboard_post_validation_sets_build_assertion, fake_dashboard_post_validation_sets, @@ -260,7 +273,7 @@ def test_edit_validation_sets_with_errors_to_amend( ).encode(), ) ), - validation_sets_payload.assertions[0], + fake_build_assertion, ] confirm_mock = mocker.patch("snapcraft.utils.confirm_with_user", return_value=True) @@ -291,7 +304,7 @@ def test_edit_validation_sets_with_errors_to_amend( ] assert fake_snap_sign.mock_calls == [ call( - validation_sets_payload.assertions[0].marshal(), + fake_build_assertion.marshal_scalars_as_strings(), key_name=None, ) ] @@ -300,7 +313,7 @@ def test_edit_validation_sets_with_errors_to_amend( @pytest.mark.usefixtures("memory_keyring", "fake_edit_validation_sets") def test_edit_validation_sets_with_errors_not_amended( - validation_sets_payload, + fake_validation_sets, fake_dashboard_get_validation_sets, fake_dashboard_post_validation_sets_build_assertion, fake_dashboard_post_validation_sets, @@ -344,20 +357,52 @@ def test_edit_validation_sets_with_errors_not_amended( def test_edit_yaml_error_retry(mocker, tmp_path, monkeypatch): monkeypatch.setenv("EDITOR", "faux-vi") + tmp_file = tmp_path / "validation_sets_template" + confirm_mock = mocker.patch("snapcraft.utils.confirm_with_user", return_value=True) + expected_edited_assertion = validation_sets.EditableBuildAssertion.unmarshal( + { + "account-id": "test", + "name": "test", + "sequence": 1, + "snaps": [{"name": "core22"}], + } + ) + good_yaml = "{'account-id': 'test', 'name': 'test', 'sequence': 1, 'snaps': [{'name': 'core22'}]}" + bad_yaml = f"badyaml}} {good_yaml}" + data_write = [good_yaml, bad_yaml] + + def side_effect(*args, **kwargs): + tmp_file.write_text(data_write.pop(), encoding="utf-8") + subprocess_mock = mocker.patch("subprocess.run", side_effect=side_effect) + + assert edit_validation_sets(tmp_file) == expected_edited_assertion + assert confirm_mock.mock_calls == [call("Do you wish to amend the validation set?")] + assert subprocess_mock.mock_calls == [call(["faux-vi", tmp_file], check=True)] * 2 + + +def test_edit_pydantic_error_retry(mocker, tmp_path, monkeypatch): + monkeypatch.setenv("EDITOR", "faux-vi") tmp_file = tmp_path / "validation_sets_template" confirm_mock = mocker.patch("snapcraft.utils.confirm_with_user", return_value=True) - data_write = [ - "{good: yaml}", - "{{bad yaml {{", - ] + expected_edited_assertion = validation_sets.EditableBuildAssertion.unmarshal( + { + "account-id": "test", + "name": "test", + "sequence": 1, + "snaps": [{"name": "core22"}], + } + ) + good_yaml = "{'account-id': 'test', 'name': 'test', 'sequence': 1, 'snaps': [{'name': 'core22'}]}" + bad_yaml = "{'invalid-field': 'test'}" + data_write = [good_yaml, bad_yaml] def side_effect(*args, **kwargs): tmp_file.write_text(data_write.pop(), encoding="utf-8") subprocess_mock = mocker.patch("subprocess.run", side_effect=side_effect) - assert edit_validation_sets(tmp_file) == {"good": "yaml"} + assert edit_validation_sets(tmp_file) == expected_edited_assertion assert confirm_mock.mock_calls == [call("Do you wish to amend the validation set?")] assert subprocess_mock.mock_calls == [call(["faux-vi", tmp_file], check=True)] * 2 From 6425f1c7a3faaf5d7156f22bed2ba5d332d45ff6 Mon Sep 17 00:00:00 2001 From: Aristo Chen Date: Thu, 1 Aug 2024 00:22:12 +0800 Subject: [PATCH 4/6] fix(kernel_plugin): ignoring missing files when removing symlinks (#4915) Signed-off-by: Aristo Chen --- snapcraft_legacy/plugins/v2/_kernel_build.py | 2 +- tests/legacy/unit/plugins/v2/test_kernel.py | 2 +- tests/unit/parts/plugins/test_kernel.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/snapcraft_legacy/plugins/v2/_kernel_build.py b/snapcraft_legacy/plugins/v2/_kernel_build.py index e1fd3d80c9..48ec7243b2 100644 --- a/snapcraft_legacy/plugins/v2/_kernel_build.py +++ b/snapcraft_legacy/plugins/v2/_kernel_build.py @@ -1111,7 +1111,7 @@ def _arrange_install_dir_cmd(install_dir: str) -> List[str]: # but snapd expects modules/ and firmware/ mv {install_dir}/lib/modules {install_dir}/ # remove symlinks modules/*/build and modules/*/source - rm {install_dir}/modules/*/build {install_dir}/modules/*/source + rm -f {install_dir}/modules/*/build {install_dir}/modules/*/source # if there is firmware dir, move it to snap root # this could have been from stage packages or from kernel build [ -d {install_dir}/lib/firmware ] && mv {install_dir}/lib/firmware {install_dir} diff --git a/tests/legacy/unit/plugins/v2/test_kernel.py b/tests/legacy/unit/plugins/v2/test_kernel.py index 939316c14b..0b7729ad8c 100644 --- a/tests/legacy/unit/plugins/v2/test_kernel.py +++ b/tests/legacy/unit/plugins/v2/test_kernel.py @@ -1768,7 +1768,7 @@ def _is_sub_array(array, sub_array): # but snapd expects modules/ and firmware/ mv ${SNAPCRAFT_PART_INSTALL}/lib/modules ${SNAPCRAFT_PART_INSTALL}/ # remove symlinks modules/*/build and modules/*/source - rm ${SNAPCRAFT_PART_INSTALL}/modules/*/build ${SNAPCRAFT_PART_INSTALL}/modules/*/source + rm -f ${SNAPCRAFT_PART_INSTALL}/modules/*/build ${SNAPCRAFT_PART_INSTALL}/modules/*/source # if there is firmware dir, move it to snap root # this could have been from stage packages or from kernel build [ -d ${SNAPCRAFT_PART_INSTALL}/lib/firmware ] && mv ${SNAPCRAFT_PART_INSTALL}/lib/firmware ${SNAPCRAFT_PART_INSTALL} diff --git a/tests/unit/parts/plugins/test_kernel.py b/tests/unit/parts/plugins/test_kernel.py index 0bc4a68166..09db04137b 100644 --- a/tests/unit/parts/plugins/test_kernel.py +++ b/tests/unit/parts/plugins/test_kernel.py @@ -1919,7 +1919,7 @@ def _is_sub_array(array, sub_array): # but snapd expects modules/ and firmware/ mv ${CRAFT_PART_INSTALL}/lib/modules ${CRAFT_PART_INSTALL}/ # remove symlinks modules/*/build and modules/*/source - rm ${CRAFT_PART_INSTALL}/modules/*/build ${CRAFT_PART_INSTALL}/modules/*/source + rm -f ${CRAFT_PART_INSTALL}/modules/*/build ${CRAFT_PART_INSTALL}/modules/*/source # if there is firmware dir, move it to snap root # this could have been from stage packages or from kernel build [ -d ${CRAFT_PART_INSTALL}/lib/firmware ] && mv ${CRAFT_PART_INSTALL}/lib/firmware ${CRAFT_PART_INSTALL} From 952a8e04da7fabb378b22bf8018e99af3614849b Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Thu, 1 Aug 2024 13:20:14 -0300 Subject: [PATCH 5/6] fix: fix Python venv in core24 classic snaps (#4946) --- snapcraft/parts/plugins/python_plugin.py | 35 +++++++++++++++- snapcraft/services/lifecycle.py | 12 ++++++ .../environment/test-variables/task.yaml | 2 +- .../python-hello/classic/snap/snapcraft.yaml | 3 ++ .../core24/python-hello/src/hello/__init__.py | 5 ++- .../python-hello/strict/snap/snapcraft.yaml | 2 + tests/spread/core24/python-hello/task.yaml | 7 +++- .../unit/parts/plugins/test_python_plugin.py | 40 ++++++++++++++++++- tests/unit/services/test_lifecycle.py | 10 ++++- 9 files changed, 109 insertions(+), 7 deletions(-) diff --git a/snapcraft/parts/plugins/python_plugin.py b/snapcraft/parts/plugins/python_plugin.py index fb2159a9ca..df20f19871 100644 --- a/snapcraft/parts/plugins/python_plugin.py +++ b/snapcraft/parts/plugins/python_plugin.py @@ -17,9 +17,10 @@ """The Snapcraft Python plugin.""" import logging +from pathlib import Path from typing import Optional -from craft_parts import errors +from craft_parts import StepInfo, errors from craft_parts.plugins import python_plugin from overrides import override @@ -64,3 +65,35 @@ def _get_system_python_interpreter(self) -> Optional[str]: confinement, ) return interpreter + + @classmethod + def post_prime(cls, step_info: StepInfo) -> None: + """Perform Python-specific actions right before packing.""" + base = step_info.project_base + + if base in ("core20", "core22"): + # Only fix pyvenv.cfg on core24+ snaps + return + + root_path: Path = step_info.prime_dir + + pyvenv = root_path / "pyvenv.cfg" + if not pyvenv.is_file(): + return + + snap_path = Path(f"/snap/{step_info.project_name}/current") + new_home = f"home = {snap_path}" + + candidates = ( + step_info.part_install_dir, + step_info.stage_dir, + ) + + old_contents = contents = pyvenv.read_text() + for candidate in candidates: + old_home = f"home = {candidate}" + contents = contents.replace(old_home, new_home) + + if old_contents != contents: + logger.debug("Updating pyvenv.cfg to:\n%s", contents) + pyvenv.write_text(contents) diff --git a/snapcraft/services/lifecycle.py b/snapcraft/services/lifecycle.py index 6bfae8d160..61f8f95472 100644 --- a/snapcraft/services/lifecycle.py +++ b/snapcraft/services/lifecycle.py @@ -77,6 +77,7 @@ def setup(self) -> None: extra_build_snaps=project.get_extra_build_snaps(), confinement=project.confinement, project_base=project.base or "", + project_name=project.name, ) callbacks.register_prologue(parts.set_global_environment) callbacks.register_pre_step(parts.set_step_environment) @@ -85,8 +86,19 @@ def setup(self) -> None: @overrides def post_prime(self, step_info: StepInfo) -> bool: """Run post-prime parts steps for Snapcraft.""" + from snapcraft.parts import plugins + project = cast(models.Project, self._project) + part_name = step_info.part_name + plugin_name = project.parts[part_name]["plugin"] + + # Handle plugin-specific prime fixes + if plugin_name == "python": + plugins.PythonPlugin.post_prime(step_info) + + # Handle patch-elf + # do not use system libraries in classic confinement use_system_libs = not bool(project.confinement == "classic") diff --git a/tests/spread/core24-suites/environment/test-variables/task.yaml b/tests/spread/core24-suites/environment/test-variables/task.yaml index 15c6fbae11..f9fb09d0a2 100644 --- a/tests/spread/core24-suites/environment/test-variables/task.yaml +++ b/tests/spread/core24-suites/environment/test-variables/task.yaml @@ -33,7 +33,7 @@ execute: | cat "$file" for exp in \ "^SNAPCRAFT_PROJECT_GRADE=devel$" \ - "^SNAPCRAFT_PROJECT_NAME=None$" \ + "^SNAPCRAFT_PROJECT_NAME=variables$" \ "^SNAPCRAFT_PROJECT_VERSION=1$" \ "^SNAPCRAFT_PARALLEL_BUILD_COUNT=[0-9]\+$" \ "^SNAPCRAFT_PROJECT_DIR=${root}$" \ diff --git a/tests/spread/core24/python-hello/classic/snap/snapcraft.yaml b/tests/spread/core24/python-hello/classic/snap/snapcraft.yaml index bfac77d1a7..d9d88dff4d 100644 --- a/tests/spread/core24/python-hello/classic/snap/snapcraft.yaml +++ b/tests/spread/core24/python-hello/classic/snap/snapcraft.yaml @@ -16,6 +16,8 @@ parts: hello: plugin: python source: src + python-packages: + - black build-attributes: - enable-patchelf stage-packages: @@ -23,3 +25,4 @@ parts: - libpython3.12-stdlib - python3.12-minimal - python3.12-venv + - python3-minimal # (for the "python3" symlink) diff --git a/tests/spread/core24/python-hello/src/hello/__init__.py b/tests/spread/core24/python-hello/src/hello/__init__.py index e3095b2229..de8c28b98f 100644 --- a/tests/spread/core24/python-hello/src/hello/__init__.py +++ b/tests/spread/core24/python-hello/src/hello/__init__.py @@ -1,2 +1,5 @@ +import black + + def main(): - print("hello world") + print(f"hello world! black version: {black.__version__}") diff --git a/tests/spread/core24/python-hello/strict/snap/snapcraft.yaml b/tests/spread/core24/python-hello/strict/snap/snapcraft.yaml index 2192e0e234..f52a1ca966 100644 --- a/tests/spread/core24/python-hello/strict/snap/snapcraft.yaml +++ b/tests/spread/core24/python-hello/strict/snap/snapcraft.yaml @@ -12,3 +12,5 @@ parts: hello: plugin: python source: src + python-packages: + - black diff --git a/tests/spread/core24/python-hello/task.yaml b/tests/spread/core24/python-hello/task.yaml index 11c3f18fd1..2db4043b9e 100644 --- a/tests/spread/core24/python-hello/task.yaml +++ b/tests/spread/core24/python-hello/task.yaml @@ -1,5 +1,10 @@ summary: Build and run Python-based snaps in core24 +systems: + # Must *not* run this on 24.04, which can give false-positives due to the + # presence of the system Python 3.12. + - ubuntu-22.04* + environment: PARAM/strict: "" PARAM/classic: "--classic" @@ -17,4 +22,4 @@ execute: | # shellcheck disable=SC2086 snap install python-hello-"${SPREAD_VARIANT}"_1.0_*.snap --dangerous ${PARAM} - python-hello-"${SPREAD_VARIANT}" | MATCH "hello world" + python-hello-"${SPREAD_VARIANT}" | MATCH "hello world! black version" diff --git a/tests/unit/parts/plugins/test_python_plugin.py b/tests/unit/parts/plugins/test_python_plugin.py index 36487cfcaa..c4cc654dca 100644 --- a/tests/unit/parts/plugins/test_python_plugin.py +++ b/tests/unit/parts/plugins/test_python_plugin.py @@ -17,7 +17,7 @@ from textwrap import dedent import pytest -from craft_parts import Part, PartInfo, ProjectInfo, errors +from craft_parts import Part, PartInfo, ProjectInfo, Step, StepInfo, errors from snapcraft.parts.plugins import PythonPlugin @@ -190,3 +190,41 @@ def test_get_system_python_interpreter_unknown_base(confinement, new_dir): expected_error = "Don't know which interpreter to use for base core10" with pytest.raises(errors.PartsError, match=expected_error): plugin._get_system_python_interpreter() + + +@pytest.mark.parametrize("home_attr", ["part_install_dir", "stage_dir"]) +def test_fix_pyvenv(new_dir, home_attr): + part_info = PartInfo( + project_info=ProjectInfo( + application_name="test", + project_name="test-snap", + base="core24", + confinement="classic", + project_base="core24", + cache_dir=new_dir, + ), + part=Part("my-part", {"plugin": "python"}), + ) + + prime_dir = part_info.prime_dir + prime_dir.mkdir() + + pyvenv = prime_dir / "pyvenv.cfg" + pyvenv.write_text( + dedent( + f"""\ + home = {getattr(part_info, home_attr)}/usr/bin + include-system-site-packages = false + version = 3.12.3 + executable = /root/parts/my-part/install/usr/bin/python3.12 + command = /root/parts/my-part/install/usr/bin/python3 -m venv /root/parts/my-part/install + """ + ) + ) + + step_info = StepInfo(part_info, Step.PRIME) + + PythonPlugin.post_prime(step_info) + + new_contents = pyvenv.read_text() + assert "home = /snap/test-snap/current/usr/bin" in new_contents diff --git a/tests/unit/services/test_lifecycle.py b/tests/unit/services/test_lifecycle.py index 6eb47794af..3c7d63de10 100644 --- a/tests/unit/services/test_lifecycle.py +++ b/tests/unit/services/test_lifecycle.py @@ -43,7 +43,10 @@ def test_lifecycle_installs_base(lifecycle_service, mocker): ) -def test_post_prime_no_patchelf(fp, tmp_path, lifecycle_service): +def test_post_prime_no_patchelf(fp, tmp_path, lifecycle_service, default_project): + new_attrs = {"parts": {"my-part": {"plugin": "nil"}}} + default_project.__dict__.update(**new_attrs) + mock_step_info = mock.Mock() mock_step_info.configure_mock( **{ @@ -51,6 +54,7 @@ def test_post_prime_no_patchelf(fp, tmp_path, lifecycle_service): "build_attributes": [], "state.files": ["usr/bin/ls"], "prime_dir": tmp_path / "prime", + "part_name": "my-part", } ) @@ -82,7 +86,7 @@ def test_post_prime_patchelf( use_system_libs, ): patchelf_spy = mocker.spy(snapcraft.parts, "patch_elf") - new_attrs = {"confinement": confinement} + new_attrs = {"confinement": confinement, "parts": {"my-part": {"plugin": "nil"}}} default_project.__dict__.update(**new_attrs) mock_step_info = mock.Mock() @@ -92,6 +96,7 @@ def test_post_prime_patchelf( "build_attributes": ["enable-patchelf"], "state.files": ["usr/bin/ls"], "prime_dir": tmp_path / "prime", + "part_name": "my-part", } ) @@ -207,6 +212,7 @@ def test_lifecycle_custom_arguments( assert info.project_base == expected_base assert info.confinement == expected_confinement + assert info.project_name == default_project.name == "default" @pytest.mark.usefixtures("default_project") From c12aaaf74443c1b5ed60cd1646c14e4561634746 Mon Sep 17 00:00:00 2001 From: Callahan Kovacs Date: Mon, 5 Aug 2024 11:46:53 -0500 Subject: [PATCH 6/6] docs(changelog): add 8.3.2 release notes Signed-off-by: Callahan Kovacs --- docs/reference/changelog.rst | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index c64516bea5..c473928b31 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -77,6 +77,40 @@ Changelog For a complete list of commits, check out the `X.Y.Z`_ release on GitHub. + +8.3.2 (2024-Aug-05) +------------------- + +Core +==== + +Bases +##### + +core24 +"""""" + +* Fix a bug where classic snaps with a Python virtual environment would attempt + to use the system's Python intepretter (`#4942`_). + +Plugins +####### + +Kernel +"""""" + +* Fix a bug where removing a missing symlink would cause the kernel plugin + to fail. + +Store +===== + +* Fix a bug where ``edit-validation-sets`` would fail when editing a validation + sets with snap revisions (`#4909`_). + +For a complete list of commits, check out the `8.3.2`_ release on GitHub. + + 8.3.1 (2024-Jul-08) ------------------- @@ -964,6 +998,8 @@ For a complete list of commits, check out the `8.0.0`_ release on GitHub. .. _#4886: https://github.com/canonical/snapcraft/issues/4886 .. _#4889: https://github.com/canonical/snapcraft/issues/4889 .. _#4890: https://github.com/canonical/snapcraft/issues/4890 +.. _#4909: https://github.com/canonical/snapcraft/issues/4909 +.. _#4942: https://github.com/canonical/snapcraft/issues/4942 .. _8.0.0: https://github.com/canonical/snapcraft/releases/tag/8.0.0 .. _8.0.1: https://github.com/canonical/snapcraft/releases/tag/8.0.1 @@ -987,3 +1023,4 @@ For a complete list of commits, check out the `8.0.0`_ release on GitHub. .. _8.2.12: https://github.com/canonical/snapcraft/releases/tag/8.2.12 .. _8.3.0: https://github.com/canonical/snapcraft/releases/tag/8.3.0 .. _8.3.1: https://github.com/canonical/snapcraft/releases/tag/8.3.1 +.. _8.3.2: https://github.com/canonical/snapcraft/releases/tag/8.3.2