Skip to content

Commit

Permalink
Merge branch 'fix/do_not_expand_env_variables_in_example_manifests' i…
Browse files Browse the repository at this point in the history
…nto 'main'

fix: Do not expand environment variables when validating manifest

Closes PACMAN-1035

See merge request espressif/idf-component-manager!475
  • Loading branch information
kumekay committed Dec 6, 2024
2 parents 26363a3 + 9e15c81 commit a6a052f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
8 changes: 5 additions & 3 deletions idf_component_manager/core_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
filtered_paths,
)
from idf_component_tools.hash_tools.constants import HASH_FILENAME
from idf_component_tools.manager import ManifestManager
from idf_component_tools.manager import ManifestManager, UploadMode
from idf_component_tools.manifest import Manifest
from idf_component_tools.manifest.constants import SLUG_BODY_REGEX
from idf_component_tools.semver import SimpleSpec
Expand Down Expand Up @@ -94,7 +94,9 @@ def validate_examples_manifest(path: str) -> None:
# Find all manifest files in examples directory
for manifest_path in examples_path.rglob(MANIFEST_FILENAME):
# Check if the manifest file is valid
ManifestManager(manifest_path, manifest_path.parent.parent.name).load()
ManifestManager(
manifest_path, manifest_path.parent.parent.name, upload_mode=UploadMode.example
).load()


def raise_component_modified_error(managed_components_dir: str, components: t.List[str]) -> None:
Expand Down Expand Up @@ -158,7 +160,7 @@ def parse_component_name_spec(
match = re.match(COMPONENT_FULL_NAME_WITH_SPEC_REGEX, component_name)
if not match:
raise FatalError(
'Cannot parse COMPONENT argument. ' 'Please use format like: namespace/component=1.0.0'
'Cannot parse COMPONENT argument. Please use format like: namespace/component=1.0.0'
)

namespace = match.group('namespace') or default_namespace or DEFAULT_NAMESPACE
Expand Down
3 changes: 3 additions & 0 deletions idf_component_tools/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def validate(self) -> 'ManifestManager':
from .manifest.models import (
Manifest,
RepositoryInfoField,
set_validation_context,
)

# avoid circular dependency
Expand Down Expand Up @@ -98,6 +99,8 @@ def validate(self) -> 'ManifestManager':

manifest_dict['manifest_manager'] = self

set_validation_context({'upload_mode': self.upload_mode})

self._validation_errors, self._manifest = Manifest.validate_manifest( # type: ignore
manifest_dict,
upload_mode=self.upload_mode,
Expand Down
21 changes: 20 additions & 1 deletion idf_component_tools/manifest/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0
import typing as t
import warnings
from contextvars import ContextVar
from copy import deepcopy

from pydantic import (
Expand Down Expand Up @@ -32,6 +33,7 @@
from idf_component_tools.errors import (
InternalError,
MetadataKeyError,
RunningEnvironmentError,
)
from idf_component_tools.hash_tools.calculate import hash_object
from idf_component_tools.logging import suppress_logging
Expand Down Expand Up @@ -60,6 +62,17 @@
from .constants import COMPILED_FULL_SLUG_REGEX, known_targets
from .if_parser import IfClause, parse_if_clause

# Context for model validation
_manifest_validation_context: ContextVar = ContextVar('manifest_validation_context', default={})


def set_validation_context(context: t.Dict[str, t.Any]):
_manifest_validation_context.set(context)


def get_validation_context() -> t.Dict[str, t.Any]:
return _manifest_validation_context.get()


class OptionalDependency(BaseModel):
# use alias since `if` is a keyword
Expand All @@ -79,6 +92,12 @@ def validate_if_clause(cls, v: str):
obj.get_value()
except ParseException:
raise ValueError('Invalid syntax: "{}"'.format(v))
except RunningEnvironmentError as e:
if get_validation_context().get('upload_mode') not in [
UploadMode.example,
UploadMode.false,
]:
raise e

return v

Expand Down Expand Up @@ -443,7 +462,7 @@ def validate_post_init(self) -> None:
if not self.repository and self.repository_info:
raise ValueError('Invalid field "repository". Must set when "repository_info" is set')

if self._upload_mode != UploadMode.false:
if self._upload_mode == UploadMode.component:
self._validate_while_uploading()

def model_dump(self, **kwargs) -> t.Dict[str, t.Any]:
Expand Down
16 changes: 14 additions & 2 deletions tests/idf_comonent_tools_tests/manifest/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

from idf_component_manager.core import get_validated_manifest
from idf_component_tools import LOGGING_NAMESPACE
from idf_component_tools.errors import ManifestError
from idf_component_tools.manager import ManifestManager
from idf_component_tools.errors import ManifestError, RunningEnvironmentError
from idf_component_tools.manager import ManifestManager, UploadMode


def test_check_filename(tmp_path):
Expand Down Expand Up @@ -143,3 +143,15 @@ def test_get_validated_manifest_invalid_component_manifest(valid_manifest, tmp_p

with pytest.raises(ManifestError, match='Manifest is not valid'):
get_validated_manifest(manager, tmp_path)


def test_env_not_expanded_with_example_manifest(valid_manifest, tmp_path):
valid_manifest['dependencies']['test']['rules'] = [{'if': '$MY_IDF_VAR == 1'}]
manifest_path = os.path.join(str(tmp_path), 'idf_component.yml')
with open(manifest_path, 'w') as fw:
yaml.dump(valid_manifest, fw)

with pytest.raises(RunningEnvironmentError):
ManifestManager(manifest_path, name='test', upload_mode=UploadMode.component).load()

ManifestManager(manifest_path, name='test', upload_mode=UploadMode.example).load()

0 comments on commit a6a052f

Please sign in to comment.