diff --git a/changelog/+deploy_key_actions_present.added.md b/changelog/+deploy_key_actions_present.added.md index 0247df15..87cacb91 100644 --- a/changelog/+deploy_key_actions_present.added.md +++ b/changelog/+deploy_key_actions_present.added.md @@ -1 +1 @@ -Added composite states that generate an SSH keypair in memory via the `ssh_pki` module and distribute the public half as a deploy key and the private half as an Actions secret, with optional rotation ({py:func}`github_repo.deploy_key_actions_present ` and the resource-type {py:func}`deploy_key.actions_present `) +Added composite states that generate an SSH keypair in memory via the {py:mod}`ssh_pki ` module and distribute the public half as a deploy key and the private half as an Actions secret, with optional rotation ({py:func}`github_repo.deploy_key_actions_present ` and the resource-type {py:func}`deploy_key.actions_present `) diff --git a/changelog/+grains_files.added.md b/changelog/+grains_files.added.md new file mode 100644 index 00000000..b2a2c350 --- /dev/null +++ b/changelog/+grains_files.added.md @@ -0,0 +1 @@ +Added custom repository resource grains sourced from in-repo YAML/JSON files (e.g. copier answers) via the {ghconf}`grains_files ` resource declaration key, with per-namespace key filtering, renaming and optional merging into the root grains diff --git a/changelog/+persistent_cache.added.md b/changelog/+persistent_cache.added.md new file mode 100644 index 00000000..fcd404cb --- /dev/null +++ b/changelog/+persistent_cache.added.md @@ -0,0 +1 @@ +Added a persistent cross-process per-connection cache (on by default, opt out via the {ghconf}`cache` profile parameter), serving token/account identities and file-based custom resource grains (validated against each repository's `pushed_at` timestamp) without refetching, e.g. across `salt-call --local` invocations diff --git a/changelog/+repo_info_fields.added.md b/changelog/+repo_info_fields.added.md new file mode 100644 index 00000000..493fab4a --- /dev/null +++ b/changelog/+repo_info_fields.added.md @@ -0,0 +1 @@ +Extended repository information ({py:func}`github_repo.info `/{py:func}`github_repo.list `, but also affects the legacy {py:func}`github.get_repo_info ` and the resource grains) with `visibility`, `archived`, `disabled`, `is_template`, `license`, `topics`, `custom_properties`, `clone_url`, `ssh_url`, `created_at`, `has_discussions`, `has_pages` and `has_projects` (info additionally carries `updated_at`) diff --git a/docs/ref/resources/github/saltext.github.resources.github.modules.grains.rst b/docs/ref/resources/github/saltext.github.resources.github.modules.grains.rst new file mode 100644 index 00000000..b4d66906 --- /dev/null +++ b/docs/ref/resources/github/saltext.github.resources.github.modules.grains.rst @@ -0,0 +1,5 @@ +``grains`` +========== + +.. automodule:: saltext.github.resources.github.modules.grains + :members: diff --git a/docs/ref/resources/github/saltext.github.resources.github.modules.rst b/docs/ref/resources/github/saltext.github.resources.github.modules.rst index 722722c8..8b28db2a 100644 --- a/docs/ref/resources/github/saltext.github.resources.github.modules.rst +++ b/docs/ref/resources/github/saltext.github.resources.github.modules.rst @@ -10,6 +10,7 @@ Execution Modules config deploy_key environment + grains issue label milestone diff --git a/docs/ref/utils/index.rst b/docs/ref/utils/index.rst index 9e34b946..c5810e8e 100644 --- a/docs/ref/utils/index.rst +++ b/docs/ref/utils/index.rst @@ -12,10 +12,12 @@ _________ utils utils.gh utils.gh.actions + utils.gh.cache utils.gh.client utils.gh.deploy_keys utils.gh.environments utils.gh.exceptions + utils.gh.grains_files utils.gh.issues utils.gh.labels utils.gh.members diff --git a/docs/ref/utils/saltext.github.utils.gh.cache.rst b/docs/ref/utils/saltext.github.utils.gh.cache.rst new file mode 100644 index 00000000..28216140 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.gh.cache.rst @@ -0,0 +1,5 @@ +saltext.github.utils.gh.cache +============================= + +.. automodule:: saltext.github.utils.gh.cache + :members: diff --git a/docs/ref/utils/saltext.github.utils.gh.grains_files.rst b/docs/ref/utils/saltext.github.utils.gh.grains_files.rst new file mode 100644 index 00000000..72a6c015 --- /dev/null +++ b/docs/ref/utils/saltext.github.utils.gh.grains_files.rst @@ -0,0 +1,5 @@ +saltext.github.utils.gh.grains_files +==================================== + +.. automodule:: saltext.github.utils.gh.grains_files + :members: diff --git a/docs/topics/configuration.md b/docs/topics/configuration.md index 2b09166f..782d7f37 100644 --- a/docs/topics/configuration.md +++ b/docs/topics/configuration.md @@ -21,7 +21,7 @@ The following keys can be specified in the profile: :::{ghconf} token ::: `token` -: Required. Access token to authenticate with. +: **Required**. Access token to authenticate with. :::{ghconf} owner ::: @@ -40,6 +40,44 @@ The following keys can be specified in the profile: `org_name` keeps being supported as a legacy alias. ::: +:::{ghconf} api_url +::: +`api_url` +: Base URL of the GitHub API. Set this when targeting a GitHub + Enterprise Server instance, e.g. `https://github.example.com/api/v3`. + Defaults to `https://api.github.com`. + +:::{ghconf} cache +::: +`cache` +: Keep a persistent cross-process cache for specific API data, + stored in the minion's {conf_minion}`cachedir`. + All entries are namespaced by a token/API URL-derived identity, + hence rotated credentials never serve stale data. + The data that is cached is either static or refreshed as soon + as it changes. Defaults to true. + + :::{versionadded} 1.1.0 + ::: + + :::{hint} + Always cached are: + + * {ghconf}`token`-associated user handle (never changes) + * account name => account type lookups (should never change) + + When the {py:mod}`github resource type ` is configured + with {ghconf}`grains_files `, file contents + (including negative lookups) are also cached in relation to + the repository's `pushed_at` timestamp (and automatically refreshed as soon + as that timestamp changes). + {py:func}`grains_refresh ` + bypasses and rewrites the cached file contents. + This especially benefits `salt-call --local` setups, where every + invocation starts a fresh process and would otherwise refetch all + file-based grains. + ::: + :::{ghconf} repo_name ::: `repo_name` @@ -54,13 +92,6 @@ The following keys can be specified in the profile: : Allow automated repository privacy modifications. Defaults to false since it can be dangerous to change the privacy of a repository in an automated way. -:::{ghconf} api_url -::: -`api_url` -: Base URL of the GitHub API. Set this when targeting a GitHub - Enterprise Server instance, e.g. `https://github.example.com/api/v3`. - Defaults to `https://api.github.com`. - ## Multiple profiles Any number of profiles can be defined and selected per call: @@ -179,6 +210,55 @@ the master's {conf_master}`pillar_opts` setting) – including SDB rendering. Can be overridden per owner in the {ghconf}`owners ` mapping. +:::{ghconf} resource:grains_files +::: +`grains_files` +: Custom resource grains sourced from YAML/JSON files inside each + repository. A mapping of grain namespace to file specification – + each namespace becomes a grain key next to the builtin ones + (`id`, `owner`, `repo`, ..., which namespaces must not shadow). + The special `_` namespace instead merges its keys into the root + grains level; keys that already exist – the builtin identity grains + or earlier namespaces – are never overwritten. + + `path` + : File path(s) inside the repository, a string or a list. All listed + files are fetched; missing ones are skipped and the parsed mappings + are shallow-merged in list order, earlier files winning. + + `include` + : Optional list of strings or single-key dicts selecting the exposed keys. + + String entries are fnmatch patterns matched case-sensitively against + colon-delimited key paths; a match includes the whole subtree, preserving nesting. + `*` crosses `:` boundaries. + + Single-entry mappings rename: + the value at the exact source path (scalar or subtree, no patterns) + is exposed under the new name as a flat or nested namespace key. + + A plain mapping of new name to source path is shorthand for a + list of mostly renames. Unset means everything is included. + + See the [Full example config][full-example-config] for detailed syntax examples. + + `exclude` + : Optional list of fnmatch patterns pruning matching key paths at + any depth, applied after `include`. Does not apply to renamed + keys. Note that `*` also crosses `:` boundaries. + + Can be overridden per owner in the {ghconf}`owners ` + mapping. Unlike the other keys, the override replaces the + declaration-level value entirely (no merging) – an empty mapping + thus disables custom grains for that owner. + + :::{important} + These grains are controlled by anyone who can commit to the + respective repository. Restrict the exposed keys via `include` + allowlists and keep that trust model in mind when building targeting + expressions. The builtin identity grains cannot be shadowed. + ::: + ### Minimal example config This example showcases the minimally required config to use the modules and expose repositories as Salt resources. @@ -232,20 +312,92 @@ other_enterprise_org: token: sdb://osenv/GITHUB_TOKEN owner: other-org -# Expose specific repositories as resources +# Expose select repositories as resources resources: github: + + # By default, for any owner, only expose repositories that start with `foo-`, + # including archived ones. + include_archived: true + repos: + - foo-* + + # Also try to load a file named `.github/data.json` from each repository and + # include all keys but those starting with an underscore in grains["data"] + grains_files: + data: + path: .github/data.json + exclude: + - "_*" + + # Expose repositories specifically from these accounts. owners: + + # Use the defaults above. Exposed are: my-github-handle/foo-* my-github-handle: {} + + # Exclude archived ones and a specific one. Exposed are my-org/foo-*, + # but not the one repository named my-org/foo-not-this-repo. my-org: exclude: - foo-not-this-repo + include_archived: false + + # Expose repositories matching other-org/bar-* while using a different profile. + # Don't render any custom grains from files in these repositories. other-org: profile: other_enterprise_org + grains_files: {} repos: - bar-* - include_archived: false - repos: - - foo-* - include_archived: true + + # Reset `repos` config to expose all repos. + # Showcase custom grains (in detail). + tiny-org: + # Reset repo selection to include all repos in tiny-org/*. + repos: {} + # Override the default custom file grains config completely + grains_files: + + # When a repo has a `.github/grains.json` file, load it and merge + # select keys (by "glob" match) into the repository's grains (no namespace). + # You can rename keys by passing a single-key dict + # (no globs allowed in source or target). + # Explicit renames always win a slot. + # Note: inbuilt grains can never be overridden. + _: + path: .github/grains.json + include: + - team + - status + - nested:key + - 'include-all-*' + - '*-also-these' + - 'and-*-these' + - 'and:*-these' # a glob spans nesting levels + - some_key: renamed:from:a:deeply:nested:key + - some:nested:target: renamed:from:another:deeply:nested:key + + # Try loading several paths and merge their data into the `data` namespace in grains. + # If more than one file specifies the same key, the *first* one wins. + # This one still excludes any keys that start with an underscore, but shows + # exclude/include can be specified as a simple string. + data: + path: + - .github/foo.json + - .github/bar.yml + exclude: _* + + # Namespaces can be nested. + # `include` can be a mapping as a shorthand if most of your + # includes are renames. Regular patterns are still supported + # by specifying a falsy value. + nested:target:copier: + path: .copier-answers.yml + include: + template_version: _commit + nested:target: my_question + nested_source: my:question + just_keep_this_explicit_key: + keep_glob_*:stuff: '' ``` diff --git a/docs/topics/repo_resources.md b/docs/topics/repo_resources.md index ac3bbf78..4aa41973 100644 --- a/docs/topics/repo_resources.md +++ b/docs/topics/repo_resources.md @@ -12,9 +12,12 @@ resource. Compared to the classic {py:mod}`github_repo ` modules, resources offer: -- direct targeting of repositories (`salt 'my-org/my-repo' ...`), - including grain-based targeting across all managed repositories -- per-repository grains (owner, default branch, visibility, ...) +- direct targeting of repositories (`salt 'my-org/my-repo'` / + `salt -C 'T@github:my-org/my-repo'`), including grain-based + targeting across all managed repositories (`salt -G 'default_branch:master'`) +- per-repository grains (owner, default branch, visibility, ...) - + a repository resource's grains can optionally even include data files + from the repo itself - repository-implicit execution and state modules – SLS files describe one repository's desired configuration and are applied per resource, without threading `repo_name`/`owner` through every call @@ -66,16 +69,45 @@ with the SRN `github:/`. ## Targeting ```bash -# One repository +# One repository, by ID salt 'my-org/my-repo' test.ping +# One repository, by SRN +salt -C 'T@github:my-org/my-repo' test.ping + # By resource grain -salt -G 'github:default_branch:master' repo.info +salt -G 'default_branch:master' repo.info ``` -The resource grains are provided under the `github` key: `id`, `owner`, -`repo`, `full_name`, `private`, `fork`, `default_branch`, `language`, -`html_url`, `description` and `homepage`. +The resource grains carry the repository's identity and basic metadata: +`id`, `owner`, `repo`, `full_name`, `private`, `visibility`, `fork`, +`archived`, `disabled`, `is_template`, `default_branch`, `language`, +`license`, `topics`, `custom_properties`, `html_url`, `clone_url`, +`ssh_url`, `description`, `homepage`, `created_at`, `has_discussions`, +`has_pages` and `has_projects`. + +Custom grains sourced from files inside each repository can be declared +via {ghconf}`resource:grains_files`, each namespace becoming a grain key +next to the builtin ones (the special `_` namespace merges into the root +grains, never overwriting existing keys): + +```bash +salt -G 'copier:template_version:2*' repo.info +``` + +:::{important} +Custom grains are controlled by anyone who can commit to the respective +repository. Restrict the exposed keys via `include` allowlists and +keep that trust model in mind when building targeting expressions. +::: + +:::{hint} +File-based grains and account metadata survive across processes (e.g. +`salt-call --local` invocations) in a persistent cache, which is +enabled by default (opt out per profile via {ghconf}`cache`). Cached +file contents stay valid until the respective repository receives a +new push. +::: ## Execution modules diff --git a/src/saltext/github/resources/github/__init__.py b/src/saltext/github/resources/github/__init__.py index 87b46f77..f95ca84e 100644 --- a/src/saltext/github/resources/github/__init__.py +++ b/src/saltext/github/resources/github/__init__.py @@ -24,11 +24,12 @@ - my-org The declaration keys (:ghconf:`resource:profile`, :ghconf:`resource:owners`, -:ghconf:`resource:repos`/:ghconf:`resource:exclude` patterns and -:ghconf:`resource:include_archived`) are described in the -:ref:`configuration guide `. ``owners`` -may be a mapping, overriding any of the other declaration keys per -owner:: +:ghconf:`resource:repos`/:ghconf:`resource:exclude` patterns, +:ghconf:`resource:include_archived` and :ghconf:`resource:grains_files` +for custom grains sourced from in-repo YAML/JSON files) are described in +the :ref:`configuration guide `. +``owners`` may be a mapping, overriding any of the other declaration +keys per owner:: resources: github: @@ -48,6 +49,7 @@ from salt.exceptions import CommandExecutionError +from saltext.github.utils.gh import grains_files as gh_grains_files from saltext.github.utils.gh import profile as gh_profile from saltext.github.utils.gh import repos as gh_repos @@ -102,14 +104,23 @@ def init(opts): Initialize the ``github`` resource type for this minion. Validates that all declared profiles (the default one and any - per-owner overrides) carry a token (without contacting the API) - and records the type-level state in ``__context__``. + per-owner overrides) carry a token (without contacting the API), + validates any ``grains_files`` declarations and records the + type-level state in ``__context__``. """ profiles = gh_profile.resource_profiles(opts) # Fail fast on unconfigured profiles; the connections themselves # are established lazily on first use. for profile in profiles: gh_profile.get_config_value(profile, "token", opts=opts, context=__context__) + # Fail fast on malformed custom grains declarations. + decl = gh_profile.resource_decl(opts) + gh_grains_files.validate(decl.get("grains_files")) + owners = decl.get("owners") + if isinstance(owners, dict): + for settings in owners.values(): + if isinstance(settings, dict): + gh_grains_files.validate(settings.get("grains_files")) __context__["github_resource"] = { "initialized": True, "profiles": profiles, @@ -154,8 +165,9 @@ def discover(opts): def grains(): """ - Return the grains of the current resource, a ``github`` mapping - carrying the repository's identity and basic metadata. + Return the grains of the current resource: the repository's identity + and basic metadata, merged with any custom grains sourced from + in-repo files per the declaration's ``grains_files`` key. """ resource_id = _resource_id() owner, repo_name = _split_id(resource_id) @@ -164,31 +176,55 @@ def grains(): # all of this owner's resources within one context. gh_repos.list_all(conn, owner=owner) info = gh_repos.get_info(conn, repo_name, owner=owner) - return { - "github": { - "id": resource_id, - "owner": info["owner"], - "repo": info["name"], - "full_name": info["full_name"], - "private": info["private"], - "fork": info["fork"], - "default_branch": info["default_branch"], - "language": info["language"], - "html_url": info["html_url"], - "description": info["description"], - "homepage": info["homepage"], - } + ret = { + "id": resource_id, + "owner": info["owner"], + "repo": info["name"], + "full_name": info["full_name"], + "private": info["private"], + "visibility": info["visibility"], + "fork": info["fork"], + "archived": info["archived"], + "disabled": info["disabled"], + "is_template": info["is_template"], + "default_branch": info["default_branch"], + "language": info["language"], + "license": info["license"], + "topics": info["topics"], + "custom_properties": info["custom_properties"], + "html_url": info["html_url"], + "clone_url": info["clone_url"], + "ssh_url": info["ssh_url"], + "description": info["description"], + "homepage": info["homepage"], + "created_at": info["created_at"], + "has_discussions": info["has_discussions"], + "has_pages": info["has_pages"], + "has_projects": info["has_projects"], } + spec = gh_profile.resource_owner_setting(__opts__, owner, "grains_files") + if spec: + # Revalidate: the pillar can have changed since init(). The + # identity grains must survive declaration/content problems. + try: + normalized = gh_grains_files.validate(spec) + except CommandExecutionError as err: + log.error("Invalid grains_files declaration, skipping custom grains: %s", err) + else: + custom = gh_grains_files.collect(conn, repo_name, normalized, owner=owner) + gh_grains_files.merge_grains(ret, custom) + return ret def grains_refresh(): """ Return freshly fetched grains for the current resource, bypassing - cached repository information. + cached repository information and file contents. """ owner, repo_name = _split_id(_resource_id()) conn = _conn(__opts__, owner) gh_repos.get_info(conn, repo_name, owner=owner, ignore_cache=True) + gh_repos.clear_file_cache(conn, repo_name, owner=owner) return grains() diff --git a/src/saltext/github/resources/github/modules/environment.py b/src/saltext/github/resources/github/modules/environment.py index 02cdd5d9..e91296db 100644 --- a/src/saltext/github/resources/github/modules/environment.py +++ b/src/saltext/github/resources/github/modules/environment.py @@ -33,7 +33,7 @@ def list_(*, fields=None, profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.list + salt 'my-org/my-repo' environment.list fields Only include these fields in the output. A single field name @@ -56,7 +56,7 @@ def get(name, *, profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.get release + salt 'my-org/my-repo' environment.get release name Name of the environment. @@ -87,7 +87,7 @@ def set_( .. code-block:: bash - salt 'github:my-org/my-repo' environment.set release \\ + salt 'my-org/my-repo' environment.set release \\ deployment_branch_policy='{protected_branches: false, custom_branch_policies: true}' name @@ -138,7 +138,7 @@ def remove(name, *, profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.remove release + salt 'my-org/my-repo' environment.remove release name Name of the environment. @@ -161,7 +161,7 @@ def list_branch_policies(environment, *, fields=None, profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.list_branch_policies release + salt 'my-org/my-repo' environment.list_branch_policies release environment Name of the environment. @@ -192,7 +192,7 @@ def add_branch_policy(environment, name, *, typ="branch", profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.add_branch_policy release main + salt 'my-org/my-repo' environment.add_branch_policy release main environment Name of the environment. @@ -224,7 +224,7 @@ def remove_branch_policy(environment, policy_id, *, profile=None): .. code-block:: bash - salt 'github:my-org/my-repo' environment.remove_branch_policy release 12345 + salt 'my-org/my-repo' environment.remove_branch_policy release 12345 environment Name of the environment. diff --git a/src/saltext/github/resources/github/modules/grains.py b/src/saltext/github/resources/github/modules/grains.py new file mode 100644 index 00000000..77da8d42 --- /dev/null +++ b/src/saltext/github/resources/github/modules/grains.py @@ -0,0 +1,18 @@ +""" +Re-expose all builtin read-only ``grains`` functions in the ``github`` resource context. +""" + +import salt.modules.grains +from salt.utils.functools import namespaced_function + +global_dict = globals() + +equals = namespaced_function(salt.modules.grains.equals, global_dict) +filter_by = namespaced_function(salt.modules.grains.filter_by, global_dict) +get = namespaced_function(salt.modules.grains.get, global_dict) +has_value = namespaced_function(salt.modules.grains.has_value, global_dict) +item = namespaced_function(salt.modules.grains.item, global_dict) +items = namespaced_function(salt.modules.grains.items, global_dict) +ls = namespaced_function(salt.modules.grains.ls, global_dict) + +fetch = get diff --git a/src/saltext/github/utils/gh/cache.py b/src/saltext/github/utils/gh/cache.py new file mode 100644 index 00000000..1685ed68 --- /dev/null +++ b/src/saltext/github/utils/gh/cache.py @@ -0,0 +1,126 @@ +""" +Optional persistent cross-process cache for slow-changing GitHub API +data, backed by the minion's :py:mod:`salt.cache` (``cachedir``). + +Attached to every profile-resolved +:class:`~saltext.github.utils.gh.client.GitHubConnection` as its +``store`` unless the profile opts out via :ghconf:`cache`. Particularly +useful for ``salt-call --local`` setups, where every invocation starts +a fresh process and the in-memory caches never survive. + +Two kinds of data are stored, namespaced by a token/API URL-derived +identity so rotated credentials or different endpoints never share +entries: + +* Repository file contents (fetched for + :ghconf:`resource:grains_files`), validated against the repository's + ``pushed_at`` timestamp – entries are served until the repository + receives a new push. Negative lookups (missing files) are cached the + same way. +* Metadata that is stable for a given token/owner: account types + (organization vs user) and the token's user login. These are stored + indefinitely. + +All storage errors degrade gracefully to cache misses. +""" + +import logging + +log = logging.getLogger(__name__) + +CACHE_ROOT = "saltext_github" +#: Sentinel for missing cache entries (``None`` is a valid cached value). +MISS = object() + + +class PersistentCache: + """ + ``salt.cache``-backed store, see the module docstring. + + opts + The minion configuration (``__opts__``), selecting the cache + backend and ``cachedir``. + + ident + Identity string namespacing all entries, derived from the + connection's token and API URL + (:attr:`~saltext.github.utils.gh.client.GitHubConnection.ident`). + """ + + def __init__(self, opts, ident): + # Local import: salt.cache pulls in the loader machinery, which + # not all importers of this package need. + import salt.cache # pylint: disable=import-outside-toplevel + + self._cache = salt.cache.factory(opts) + self._base = f"{CACHE_ROOT}/{ident}" + + def _fetch(self, bank, key): + try: + return self._cache.fetch(f"{self._base}/{bank}", key) + except Exception as err: # pylint: disable=broad-except + log.warning("Failed reading from the persistent GitHub cache: %s", err) + return None + + def _store(self, bank, key, value): + try: + self._cache.store(f"{self._base}/{bank}", key, value) + except Exception as err: # pylint: disable=broad-except + log.warning("Failed writing to the persistent GitHub cache: %s", err) + + def get_meta(self, key): + """ + Return a stored metadata value, or :data:`MISS`. + """ + data = self._fetch("meta", key) + if isinstance(data, dict) and "value" in data: + return data["value"] + return MISS + + def set_meta(self, key, value): + """ + Store a metadata value. + """ + self._store("meta", key, {"value": value}) + + def get_file(self, owner, repo, path, pushed_at): + """ + Return a repository file's stored content (which is ``None`` for + cached negative lookups), or :data:`MISS` when there is no entry + or the stored entry does not match ``pushed_at``. + """ + if not pushed_at: + return MISS + entry = self._fetch(f"files/{owner.lower()}", repo.lower()) + if ( + isinstance(entry, dict) + and entry.get("pushed_at") == pushed_at + and path in entry.get("files", {}) + ): + return entry["files"][path] + return MISS + + def set_file(self, owner, repo, path, content, pushed_at): + """ + Store a repository file's content (``None`` for a negative + lookup) under the repository's current ``pushed_at``. Entries + stored for a different ``pushed_at`` are discarded. + """ + if not pushed_at: + return + bank = f"files/{owner.lower()}" + key = repo.lower() + entry = self._fetch(bank, key) + if not isinstance(entry, dict) or entry.get("pushed_at") != pushed_at: + entry = {"pushed_at": pushed_at, "files": {}} + entry["files"][path] = content + self._store(bank, key, entry) + + def flush_repo(self, owner, repo): + """ + Drop all stored file contents of one repository. + """ + try: + self._cache.flush(f"{self._base}/files/{owner.lower()}", repo.lower()) + except Exception as err: # pylint: disable=broad-except + log.warning("Failed flushing the persistent GitHub cache: %s", err) diff --git a/src/saltext/github/utils/gh/client.py b/src/saltext/github/utils/gh/client.py index 4ec8f16c..da67c097 100644 --- a/src/saltext/github/utils/gh/client.py +++ b/src/saltext/github/utils/gh/client.py @@ -8,7 +8,9 @@ execution module and future resource modules. """ +import copy import functools +import hashlib import json import logging import re @@ -16,11 +18,14 @@ from dataclasses import InitVar from dataclasses import dataclass from dataclasses import field +from types import EllipsisType import github from github.GithubException import UnknownObjectException from salt.exceptions import CommandExecutionError +from saltext.github.utils.gh.cache import MISS +from saltext.github.utils.gh.cache import PersistentCache from saltext.github.utils.gh.exceptions import GitHubApiError from saltext.github.utils.gh.exceptions import GitHubNotFoundError @@ -50,6 +55,20 @@ def _owner_default(*args, owner=..., **kwargs): return _owner_default +def get_connection_ident(token, api_url): + """ + Render the internal identity for a (token, api_url) combination. + The output is used in context cache keys and persistent cache banks. + + token + API token from profile. + + api_url + API URL from profile. + """ + return hashlib.sha256(f"{token}:{api_url}".encode()).hexdigest()[:16] + + @dataclass(slots=True) class GitHubConnection: """ @@ -68,22 +87,63 @@ class GitHubConnection: api_url Base URL of the GitHub API. Set this when targeting a GitHub Enterprise Server instance. Defaults to the public GitHub API. + + store + Optional :class:`~saltext.github.utils.gh.cache.PersistentCache` + for slow-changing API data, surviving across processes. """ token: InitVar[str] _owner: str | None = None _: KW_ONLY api_url: str = API_URL_DEFAULT - cache: dict = field(default_factory=dict) + client: github.Github = field(init=False, repr=False) + #: Identity of the connection's credentials/endpoint, safe to expose + #: (e.g. as a persistent cache namespace). + ident: str = field(init=False, default="") + store: PersistentCache | None = field(default=None, repr=False) def __post_init__(self, token: str): self.api_url = (self.api_url or API_URL_DEFAULT).rstrip("/") + self.ident = get_connection_ident(token, self.api_url) self.client = github.Github( auth=github.Auth.Token(token), base_url=self.api_url, per_page=100 ) + def with_( + self, + *, + owner: str | None | EllipsisType = ..., + store: PersistentCache | None | EllipsisType = ..., + ): + """ + Return an instance of this connection with different internal config. + + owner + If specified, ensure the returned connection defaults to this owner. + + store + If specified, ensure the returned connection uses this persistent + store/does not use any. + """ + if owner is ... and store is ...: + return self + # `token` is an InitVar, so a replace() does not work. + # We're intentionally sharing the same client and context cache. + # Implies that sharing the same token+api_url in profiles + # with different cache config and accessing them in the same run + # can return cached data/omit writing it because of the shared context cache. + # There is no risk of data leak because it's the same token and API though. + new = copy.copy(self) + + if owner is not ...: + new._owner = owner + if store is not ...: + new.store = store + return new + @property def owner(self): """ @@ -92,7 +152,7 @@ def owner(self): """ if self._owner is None: try: - self._owner = self.auth_user.login + self._owner = self.auth_login except github.GithubException as err: raise CommandExecutionError( "Failed to look up the token's user for the default repository " @@ -130,15 +190,40 @@ def auth_user(self): self.cache["auth_user"] = self.client.get_user() return self.cache["auth_user"] + @property + def auth_login(self): + """ + Login of the token's user (cached, persistently when the + connection carries a store). + """ + if "auth_login" not in self.cache: + login = MISS + if self.store is not None: + login = self.store.get_meta("auth_login") + if login is MISS: + login = self.auth_user.login + if self.store is not None: + self.store.set_meta("auth_login", login) + self.cache["auth_login"] = login + return self.cache["auth_login"] + def owner_type(self, owner=None): """ Return the account type of an owner login, either ``Organization`` - or ``User`` (cached). Defaults to the connection's owner. + or ``User`` (cached, persistently when the connection carries a + store). Defaults to the connection's owner. """ owner = owner or self.owner key = ("owner_type", owner.lower()) if key not in self.cache: - self.cache[key] = self.client.get_user(owner).type + otype = MISS + if self.store is not None: + otype = self.store.get_meta(f"owner_type_{owner.lower()}") + if otype is MISS: + otype = self.client.get_user(owner).type + if self.store is not None: + self.store.set_meta(f"owner_type_{owner.lower()}", otype) + self.cache[key] = otype return self.cache[key] def bind(self, mod): diff --git a/src/saltext/github/utils/gh/grains_files.py b/src/saltext/github/utils/gh/grains_files.py new file mode 100644 index 00000000..9728132d --- /dev/null +++ b/src/saltext/github/utils/gh/grains_files.py @@ -0,0 +1,366 @@ +""" +Custom repository grains sourced from in-repo YAML/JSON files, as +declared via the :ghconf:`resource:grains_files` key of the ``github`` +resource type. + +A declaration maps grain namespaces to file specifications:: + + grains_files: + copier: + path: .copier-answers.yml + exclude: + - "_*" + data: + path: + - .github/metadata.json + - .github/other-metadata.json + include: + - team + - org:config + - service_tier: service:tier + +``include``/``exclude`` entries are :py:mod:`fnmatch` patterns matched +case-sensitively against colon-delimited key paths, the same idiom Salt +uses for grain targeting. Note that ``*`` is not segment-aware (it can +cross ``:`` boundaries) and keys containing ``:`` cannot be addressed. +Mapping entries in ``include`` rename: the value at the exact source +path (scalar or subtree) is exposed under the new name as a flat +namespace key. Renames are not subject to ``exclude``. A plain mapping +as the ``include`` value is shorthand for a rename-only list. + +The special ``_`` namespace merges its keys into the root grains level +instead of below a namespace key. Custom grains never overwrite +existing ones – the builtin identity grains and earlier namespaces +always win. +""" + +import copy +import fnmatch +import json +import logging + +import salt.utils.data +import salt.utils.dictupdate +import salt.utils.yaml +import yaml +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import repos as gh_repos +from saltext.github.utils.gh.exceptions import GitHubApiError + +log = logging.getLogger(__name__) + +#: Grain keys provided by the resource type itself. ``grains_files`` +#: namespaces must not shadow them — the resource identity is not +#: overridable by repository contents. +RESERVED_KEYS = frozenset( + { + "id", + "owner", + "repo", + "full_name", + "private", + "visibility", + "fork", + "archived", + "disabled", + "is_template", + "default_branch", + "language", + "license", + "topics", + "custom_properties", + "html_url", + "clone_url", + "ssh_url", + "description", + "homepage", + "created_at", + "has_discussions", + "has_pages", + "has_projects", + } +) + +_SPEC_KEYS = frozenset({"path", "include", "exclude"}) + +#: Sentinel for failed path traversal (``None`` is a valid value). +_MISSING = object() + + +def _invalid(namespace, msg): + return CommandExecutionError(f"Invalid grains_files namespace '{namespace}': {msg}") + + +def validate(spec): + """ + Normalize and validate a ``grains_files`` declaration value. + + Returns the normalized form, a mapping of namespace to + ``{"path": [str, ...], "include": [str | (new_name, source), ...] | None, + "exclude": [str, ...]}``. Raises a ``CommandExecutionError`` when the + declaration is malformed. + + spec + The declaration value. ``None`` is treated as unset. + """ + if spec is None: + return {} + if not isinstance(spec, dict): + raise CommandExecutionError( + "grains_files must be a mapping of namespace to file specification" + ) + normalized = {} + for namespace, entry in spec.items(): + if not isinstance(namespace, str) or not namespace: + raise CommandExecutionError( + f"grains_files namespaces must be non-empty strings, got: {namespace!r}" + ) + if namespace in RESERVED_KEYS: + raise _invalid(namespace, "shadows a builtin resource grain") + if not isinstance(entry, dict): + raise _invalid(namespace, "the file specification must be a mapping") + if unknown := set(entry) - _SPEC_KEYS: + raise _invalid(namespace, f"unknown key(s): {', '.join(sorted(unknown))}") + paths = entry.get("path") + if isinstance(paths, str): + paths = [paths] + if ( + not isinstance(paths, list) + or not paths + or not all(isinstance(path, str) and path for path in paths) + ): + raise _invalid(namespace, "'path' is required and must be a string or list of strings") + normalized[namespace] = { + "path": paths, + "include": _validate_include(namespace, entry.get("include")), + "exclude": _validate_exclude(namespace, entry.get("exclude")), + } + return normalized + + +def _validate_include(namespace, include): + if include is None: + return None + if isinstance(include, str): + include = [include] + elif isinstance(include, dict): + # Mapping shorthand for rename includes (unset values still imply regular inclusions) + include = [ + {new_name: source} if source else new_name for new_name, source in include.items() + ] + elif not isinstance(include, list): + raise _invalid(namespace, "'include' must be a list, string or mapping") + res = [] + for item in include: + if isinstance(item, str) and item: + res.append(item) + continue + if isinstance(item, dict) and len(item) == 1: + ((new_name, source),) = item.items() + if isinstance(new_name, str) and new_name and isinstance(source, str) and source: + res.append((new_name, source)) + continue + raise _invalid( + namespace, + "'include' entries must be patterns or single-entry " + f"'new_name: source:path' renames, got: {item!r}", + ) + return res + + +def _validate_exclude(namespace, exclude): + if exclude is None: + return [] + if isinstance(exclude, str): + return [exclude] + if not isinstance(exclude, list) or not all(isinstance(pat, str) and pat for pat in exclude): + raise _invalid(namespace, "'exclude' must be a list of patterns") + return list(exclude) + + +def load_mapping(text, *, path): + """ + Parse a file's text as YAML (which includes JSON) into a mapping of + JSON-plain values, or ``None`` (logging a warning) when the text does + not parse or does not represent a mapping. + + text + The file content. + + path + Identifier of the file's origin, used in log messages. + """ + try: + parsed = salt.utils.yaml.safe_load(text) + except yaml.YAMLError as err: + log.warning("Failed parsing '%s' as YAML/JSON: %s", path, err) + return None + if not isinstance(parsed, dict): + log.warning("File '%s' does not contain a mapping, skipping", path) + return None + try: + # Normalize to JSON-plain types (e.g. YAML timestamps -> strings) + return json.loads(json.dumps(parsed, default=str)) + except (TypeError, ValueError) as err: + log.warning("Failed normalizing the contents of '%s': %s", path, err) + return None + + +def filter_content(data, include=None, exclude=None): + """ + Apply a normalized (per :func:`validate`) include/exclude + specification to a parsed file mapping and return the result. + + data + The parsed file content. + + include + Normalized include entries: fnmatch patterns against + colon-delimited key paths (a match includes the whole subtree, + preserving nesting) and ``(new_name, source)`` rename tuples + (exact source path, value exposed as flat ``new_name`` key). + ``None`` includes everything. + + exclude + Fnmatch patterns pruning matching key paths at any depth after + includes. Does not apply to renames. + """ + patterns = None + # need to make sure the first rename wins for consistency + renames = {} + + if include is not None: + patterns = [] + for item in include: + if isinstance(item, str): + patterns.append(item) + else: + new_name, source = item + if new_name not in renames: + renames[new_name] = source + result = _filter_node(data, "", patterns, exclude or [], patterns is None) + for new_name, source in renames.items(): + source = source or new_name # unnormalized mapping-style include with no rename intention + value = salt.utils.data.traverse_dict_and_list(data, source, ..., delimiter=":") + if value is not ...: + salt.utils.dictupdate.set_dict_key_value( + result, new_name, copy.deepcopy(value), delimiter=":" + ) + return result + + +def _filter_node(node, prefix, patterns, excludes, included_parent): + result = {} + for key, value in node.items(): + path = f"{prefix}:{key}" if prefix else str(key) + if any(fnmatch.fnmatchcase(path, pat) for pat in excludes): + continue + included = included_parent or any(fnmatch.fnmatchcase(path, pat) for pat in patterns or ()) + if isinstance(value, dict): + # Recurse into non-included subtrees as well since deeper + # paths can still match an include pattern. + sub = _filter_node(value, path, patterns, excludes, included) + if included or sub: + result[key] = sub + elif included: + result[key] = copy.deepcopy(value) + return result + + +def merge_grains(grains, custom): + """ + Merge collected (per :func:`collect`) custom grains into a grains + mapping and return it. The special ``_`` namespace contributes its + keys at the root level, other namespaces become a single key each. + Keys already present – the builtin identity grains or earlier + namespaces – are never overwritten (logged and skipped). + + grains + The grains mapping to merge into (mutated). + + custom + Mapping of namespace to grain data. + """ + for namespace, data in custom.items(): + items = data.items() if namespace == "_" else ((namespace, data),) + for key, value in items: + ns_its = key.split(":") + ns_its.reverse() + + tgt = grains + ns_it = ns_its.pop() + while ns_its: + if ns_it not in tgt: + tgt[ns_it] = {} + elif not isinstance(tgt[ns_it], dict): + log.warning("Custom grain '%s' shadows an existing grain, skipping", key) + break + tgt = tgt[ns_it] + ns_it = ns_its.pop() + else: + if ns_it not in tgt: + tgt[ns_it] = value + continue + if not isinstance(tgt[ns_it], dict) or not isinstance(value, dict): + log.warning("Custom grain '%s' shadows an existing grain, skipping", key) + continue + for k, v in value.items(): + if k not in tgt[ns_it]: + tgt[ns_it][k] = v + continue + log.warning("Custom grain '%s:%s' shadows an existing grain, skipping", key, k) + return grains + + +def collect(conn, repo_name, spec, *, owner=None): + """ + Fetch, parse, merge and filter all namespaces of a normalized (per + :func:`validate`) ``grains_files`` specification for one repository. + + Returns a mapping of namespace to grain data; namespaces without any + resulting data are omitted. Per-file and per-namespace problems are + logged and skipped, never raised. + + conn + :class:`~saltext.github.utils.gh.client.GitHubConnection` to use. + + repo_name + Repository name. + + spec + The normalized ``grains_files`` specification. + + owner + Owner of the repository. Defaults to the connection's owner. + """ + owner = owner or conn.owner + ret = {} + for namespace, entry in spec.items(): + try: + merged = {} + for path in entry["path"]: + try: + content = gh_repos.get_file_content(conn, repo_name, path, owner=owner) + except GitHubApiError as err: + log.warning("Failed fetching '%s' from %s/%s: %s", path, owner, repo_name, err) + continue + if content is None: + continue + parsed = load_mapping(content, path=f"{owner}/{repo_name}:{path}") + if parsed is not None: + merged.update((k, v) for k, v in parsed.items() if k not in merged) + if not merged: + continue + data = filter_content(merged, include=entry["include"], exclude=entry["exclude"]) + if data: + ret[namespace] = data + except Exception as err: # pylint: disable=broad-except + log.warning( + "Failed collecting custom grains namespace '%s' for %s/%s: %s", + namespace, + owner, + repo_name, + err, + ) + return ret diff --git a/src/saltext/github/utils/gh/profile.py b/src/saltext/github/utils/gh/profile.py index 7424a3ad..ab8b46af 100644 --- a/src/saltext/github/utils/gh/profile.py +++ b/src/saltext/github/utils/gh/profile.py @@ -13,13 +13,14 @@ before the minion's module loader exists). """ -import hashlib import logging from salt.exceptions import CommandExecutionError +from saltext.github.utils.gh.cache import PersistentCache from saltext.github.utils.gh.client import API_URL_DEFAULT from saltext.github.utils.gh.client import GitHubConnection +from saltext.github.utils.gh.client import get_connection_ident log = logging.getLogger(__name__) @@ -161,6 +162,10 @@ def get_conn(profile, *, opts, context, utils=None): Return a :class:`~saltext.github.utils.gh.client.GitHubConnection` for a profile, cached in ``context`` for performance. + Unless the profile disables it (:ghconf:`cache`), the connection + carries a :class:`~saltext.github.utils.gh.cache.PersistentCache` + for slow-changing API data, surviving across processes. + profile Profile name that contains configuration information. @@ -174,24 +179,36 @@ def get_conn(profile, *, opts, context, utils=None): utils Pass ``__utils__`` when available (SDB rendering). """ + profile_key = f"github.profile.{profile}" + if profile_key in context: + return context[profile_key] + token = get_config_value(profile, "token", opts=opts, context=context, utils=utils) - owner = get_owner(profile, opts=opts, context=context, utils=utils) api_url = get_api_url(profile, opts=opts, context=context, utils=utils) - return _conn_for(token, owner, api_url, context) - - -def _conn_for(token, owner, api_url, context): - """ - Return the connection for the resolved profile values, cached in - ``context`` for performance. - """ # Hash the token to keep the secret out of context dumps - token_hash = hashlib.sha256(token.encode()).hexdigest()[:16] - key = f"github.{token_hash}:{owner}:{api_url}" + ident = get_connection_ident(token, api_url) + conn_key = f"github.conn.{ident}" + + # This config is HTTP connection-independent. We should reuse a client, if it exists. + owner = get_owner(profile, opts=opts, context=context, utils=utils) + cache_enabled = get_config_value( + profile, "cache", opts=opts, context=context, utils=utils, default=True + ) - if key not in context: - context[key] = GitHubConnection(token, owner, api_url=api_url) - return context[key] + if conn_key in context: + other_conn = context[conn_key] + store = ... + if bool(other_conn.store) is not cache_enabled: + store = PersistentCache(opts, ident) if cache_enabled else None + context[profile_key] = context[conn_key].with_(owner=owner, store=store) + else: + context[profile_key] = context[conn_key] = GitHubConnection( + token, + owner, + api_url=api_url, + store=cache_enabled and PersistentCache(opts, ident) or None, + ) + return context[profile_key] def resource_decl(opts): diff --git a/src/saltext/github/utils/gh/repos.py b/src/saltext/github/utils/gh/repos.py index c404c320..59270143 100644 --- a/src/saltext/github/utils/gh/repos.py +++ b/src/saltext/github/utils/gh/repos.py @@ -3,6 +3,7 @@ organization-scoped, everything else works on any owner's repositories. """ +import base64 import logging import time @@ -10,6 +11,7 @@ from salt.exceptions import CommandExecutionError from saltext.github.utils import filter_unset +from saltext.github.utils.gh.cache import MISS from saltext.github.utils.gh.client import api_request from saltext.github.utils.gh.exceptions import GitHubNotFoundError @@ -28,19 +30,34 @@ def to_dict(repo): ret["full_name"] = repo.full_name ret["owner"] = repo.owner.login ret["private"] = repo.private + ret["visibility"] = repo.visibility ret["html_url"] = repo.html_url + ret["clone_url"] = repo.clone_url + ret["ssh_url"] = repo.ssh_url ret["description"] = repo.description ret["fork"] = repo.fork + ret["archived"] = repo.archived + ret["disabled"] = repo.disabled + ret["is_template"] = repo.is_template ret["homepage"] = repo.homepage + ret["license"] = repo.license.spdx_id if repo.license else None + ret["topics"] = repo.topics + ret["custom_properties"] = repo.custom_properties ret["size"] = repo.size ret["stargazers_count"] = repo.stargazers_count ret["language"] = repo.language ret["forks_count"] = repo.forks_count ret["open_issues_count"] = repo.open_issues_count ret["default_branch"] = repo.default_branch + ret["created_at"] = repo.created_at.isoformat() if repo.created_at else None + ret["updated_at"] = repo.updated_at.isoformat() if repo.updated_at else None + ret["pushed_at"] = repo.pushed_at.isoformat() if repo.pushed_at else None + ret["has_discussions"] = repo.has_discussions + ret["has_downloads"] = repo.has_downloads ret["has_issues"] = repo.has_issues + ret["has_pages"] = repo.has_pages + ret["has_projects"] = repo.has_projects ret["has_wiki"] = repo.has_wiki - ret["has_downloads"] = repo.has_downloads return ret @@ -66,7 +83,7 @@ def list_all(conn, *, owner=None, ignore_cache=False): if key not in conn.cache or ignore_cache: if conn.owner_type(owner) == "Organization": found = conn.get_org(owner).get_repos() - elif owner.lower() == conn.auth_user.login.lower(): + elif owner.lower() == conn.auth_login.lower(): # Listing the authenticated user's repos directly includes # private ones, which /users/{owner}/repos does not carry. found = conn.auth_user.get_repos(affiliation="owner") @@ -119,6 +136,106 @@ def get_info(conn, name, *, owner=None, ignore_cache=False): return conn.cache[key] +def get_file_content(conn, name, path, *, owner=None, ignore_cache=False): + """ + Return the decoded text content of a repository file via the + contents API, or ``None`` when the file is missing or unusable + (not a regular file, too large for the API to carry inline, not + valid UTF-8). Results are cached, including negative ones – also + persistently when the connection carries a store, validated against + the repository's ``pushed_at`` timestamp. + + conn + :class:`~saltext.github.utils.gh.client.GitHubConnection` to use. + + name + Repository name. + + path + Path of the file inside the repository. + + owner + Owner of the repository. Defaults to the connection's owner. + + ignore_cache + Bypass the cached file content. + """ + owner = owner or conn.owner + key = ("repo_file", owner.lower(), name.lower(), path) + if key in conn.cache and not ignore_cache: + return conn.cache[key] + content = MISS + pushed_at = None + if conn.store is not None: + pushed_at = _pushed_at(conn, name, owner) + if not ignore_cache: + content = conn.store.get_file(owner, name, path, pushed_at) + if content is MISS: + content = _fetch_file_content(conn, owner, name, path) + if conn.store is not None: + conn.store.set_file(owner, name, path, content, pushed_at) + conn.cache[key] = content + return content + + +def _pushed_at(conn, name, owner): + """ + Return a repository's last-push timestamp (from the cached repository + information, usually pre-warmed by a listing), or ``None`` when it + cannot be determined. + """ + try: + return get_info(conn, name, owner=owner).get("pushed_at") + except CommandExecutionError: + return None + + +def _fetch_file_content(conn, owner, name, path): + try: + res = conn.repo_query(owner, name, "contents", path) + except GitHubNotFoundError: + return None + if not isinstance(res, dict) or res.get("type") != "file": + log.warning("Path '%s' in %s/%s is not a file, skipping", path, owner, name) + return None + if res.get("encoding") != "base64": + log.warning( + "Cannot decode '%s' in %s/%s: unsupported encoding '%s'", + path, + owner, + name, + res.get("encoding"), + ) + return None + try: + return base64.b64decode(res.get("content") or "").decode("utf-8") + except (ValueError, UnicodeDecodeError) as err: + log.warning("Failed decoding the content of '%s' in %s/%s: %s", path, owner, name, err) + return None + + +def clear_file_cache(conn, name, *, owner=None): + """ + Drop all cached file contents of one repository, including cached + negative lookups and persistently stored ones. + + conn + :class:`~saltext.github.utils.gh.client.GitHubConnection` to use. + + name + Repository name. + + owner + Owner of the repository. Defaults to the connection's owner. + """ + owner = owner or conn.owner + prefix = ("repo_file", owner.lower(), name.lower()) + for key in [k for k in conn.cache if isinstance(k, tuple) and k[:3] == prefix]: + del conn.cache[key] + if conn.store is not None: + conn.store.flush_repo(owner, name) + + @api_request def get_teams(conn, name, *, owner=None): """ diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 9faf6b3e..891e8dad 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -171,6 +171,9 @@ def github_profile(fake_github, fake_org): "api_url": fake_github.url, "allow_repo_privacy_changes": True, "repo_name": "repo-a", + # The persistent cache (on by default) would outlive the + # per-test fake state reset; dedicated tests re-enable it. + "cache": False, } @@ -190,7 +193,7 @@ def fake_state(fake_github): @pytest.fixture(scope="module") -def fake_org(fake_state, fake_setup): # pylint: disable=unused-argument +def fake_org(fake_state, fake_state_setup): # pylint: disable=unused-argument """ Fake organization named ``testorg``. Ensure ``fake_setup`` creates it. """ @@ -198,7 +201,7 @@ def fake_org(fake_state, fake_setup): # pylint: disable=unused-argument @pytest.fixture(scope="module") -def fake_user(fake_state, fake_setup): # pylint: disable=unused-argument +def fake_user(fake_state, fake_state_setup): # pylint: disable=unused-argument """ Fake user ``user1``, which the fake API treats as the token's (authenticated) user. Ensure ``fake_setup`` creates it. @@ -209,10 +212,13 @@ def fake_user(fake_state, fake_setup): # pylint: disable=unused-argument @pytest.fixture(autouse=True) def _restore_fake_state(fake_github): """ - Before each test, reset the state to the module's defaults as specified in + After each test, reset the state to the module's defaults as specified in ``fake_setup``. Also, clear the request log. """ - fake_github.reset() + try: + yield + finally: + fake_github.reset() @pytest.fixture(scope="module") @@ -243,6 +249,21 @@ def fake_setup(fake_state): ) release.add_branch_policy("main") repo_a.add_label("bug", color="d73a4a", description="Something is broken") + # Repository files, consumed by the resource type's grains_files tests + repo_a.add_file( + ".copier-answers.yml", + "_commit: v1.2.3\n_src_path: gh:org/tpl\nproject_name: Repo A\nteam: platform\n", + ) + repo_a.add_file( + ".github/metadata.json", + '{"team": "json-team", "service": {"tier": "gold", "name": "svc-a"},' + ' "org": {"config": {"visibility": "public"}}}', + ) + repo_a.add_file( + ".github/other-metadata.json", + # `owner` attempts to shadow a builtin identity grain (must lose) + '{"team": "other-team", "extra": true, "owner": "evil"}', + ) repo_b.set_pages(build_type="legacy", source={"branch": "gh-pages", "path": "/"}) bug = repo_a.add_issue( "Found a bug", user1, labels=["bug", "prio:high"], assignee="user2", milestone="v1.0" @@ -254,6 +275,7 @@ def fake_setup(fake_state): # User-owned repositories, incl. one shadowing an org repo's name user1.add_repo("repo-a", description="User-owned repo-a") user_repo = user1.add_repo("user-repo", private=True) + user_repo.add_file(".copier-answers.yml", "_commit: u9.9.9\nproject_name: User Repo\n") user_repo.add_label("user-bug", color="123456") user_repo.add_milestone("u1.0", user1) user_repo.add_issue("User issue", user2) @@ -266,7 +288,10 @@ def fake_setup(fake_state): @pytest.fixture(scope="module", autouse=True) -def fake_state_setup(fake_state, request): +def fake_state_reset(fake_state): fake_state.reset() - request.getfixturevalue("fake_setup") + + +@pytest.fixture(scope="module", autouse=True) +def fake_state_setup(fake_state, fake_setup): # pylint: disable=unused-argument fake_state.snapshot() diff --git a/tests/functional/modules/github/test_profiles.py b/tests/functional/modules/github/test_profiles.py index 26745c75..c9e00e34 100644 --- a/tests/functional/modules/github/test_profiles.py +++ b/tests/functional/modules/github/test_profiles.py @@ -24,8 +24,17 @@ def other_github(): @pytest.fixture(scope="module") def minion_config_defaults(github_profile, other_github): - no_url = {"token": "other-token", "owner": "testorg"} # we'll patch api_url for this one - api_url_set = {"token": "other-token", "api_url": other_github.url, "owner": "testorg"} + no_url = { # we'll patch api_url for this one + "token": "other-token", + "owner": "testorg", + "cache": False, + } + api_url_set = { + "token": "other-token", + "api_url": other_github.url, + "owner": "testorg", + "cache": False, + } broken = github_profile.copy() broken["org_name"] = "missing-org" return { diff --git a/tests/functional/modules/github/test_rulesets.py b/tests/functional/modules/github/test_rulesets.py index fe981a28..3e85ba33 100644 --- a/tests/functional/modules/github/test_rulesets.py +++ b/tests/functional/modules/github/test_rulesets.py @@ -9,6 +9,7 @@ def minion_config_defaults(github_profile): owner_profile = { "token": github_profile["token"], "owner": github_profile["org_name"], + "cache": False, } if api_url := github_profile.get("api_url"): owner_profile["api_url"] = api_url diff --git a/tests/functional/modules/github_repo/test_owner.py b/tests/functional/modules/github_repo/test_owner.py index e37045e3..aec0d5c9 100644 --- a/tests/functional/modules/github_repo/test_owner.py +++ b/tests/functional/modules/github_repo/test_owner.py @@ -17,9 +17,11 @@ def minion_config_defaults(github_profile): "token": github_profile["token"], "owner": "user1", "repo_name": "user-repo", + "cache": False, } noowner_profile = { "token": github_profile["token"], + "cache": False, } if api_url := github_profile.get("api_url"): user_profile["api_url"] = noowner_profile["api_url"] = api_url diff --git a/tests/functional/resources/modules/test_grains.py b/tests/functional/resources/modules/test_grains.py new file mode 100644 index 00000000..16e5639f --- /dev/null +++ b/tests/functional/resources/modules/test_grains.py @@ -0,0 +1,69 @@ +import pytest + +GRAINS = { + "spongebob": "squarepants", + "nested": {"look": {"here": "found"}}, + "roles": ["web", "db"], + "os_family": "RedHat", +} + + +@pytest.fixture(scope="module") +def minion_config_overrides(): + # Static grains end up in opts["grains"], which the resource loader + # packs as __grains__ into the overlay module. + return {"grains": GRAINS} + + +def test_get(res_modules, for_resource): + with for_resource("testorg/repo-a"): + assert res_modules["grains.get"]("spongebob") == "squarepants" + assert res_modules["grains.get"]("nested:look:here") == "found" + assert res_modules["grains.get"]("missing", default="fallback") == "fallback" + + +def test_fetch_aliases_get(res_modules, for_resource): + with for_resource("testorg/repo-a"): + assert res_modules["grains.fetch"]("nested:look:here") == "found" + + +def test_has_value(res_modules, for_resource): + with for_resource("testorg/repo-a"): + assert res_modules["grains.has_value"]("nested:look:here") is True + assert res_modules["grains.has_value"]("missing") is False + + +def test_items(res_modules, for_resource): + with for_resource("testorg/repo-a"): + items = res_modules["grains.items"]() + for key, val in GRAINS.items(): + assert items[key] == val + + +def test_item(res_modules, for_resource): + with for_resource("testorg/repo-a"): + res = res_modules["grains.item"]("spongebob", "os_family") + assert res == {"spongebob": "squarepants", "os_family": "RedHat"} + + +def test_ls(res_modules, for_resource): + with for_resource("testorg/repo-a"): + assert set(res_modules["grains.ls"]()) >= set(GRAINS) + + +def test_equals(res_modules, for_resource): + with for_resource("testorg/repo-a"): + assert res_modules["grains.equals"]("spongebob", "squarepants") is True + assert res_modules["grains.equals"]("spongebob", "patrick") is False + + +def test_filter_by(res_modules, for_resource): + lookup = {"RedHat": {"pkg": "dnf"}, "default": {"pkg": "apt"}} + with for_resource("testorg/repo-a"): + assert res_modules["grains.filter_by"](lookup) == {"pkg": "dnf"} + assert res_modules["grains.filter_by"](lookup, grain="missing") == {"pkg": "apt"} + + +def test_write_functions_are_not_exposed(res_modules): + for func in ("setval", "setvals", "append", "remove", "delkey", "delval", "set"): + assert f"grains.{func}" not in res_modules diff --git a/tests/functional/resources/test_connection.py b/tests/functional/resources/test_connection.py index 0d1d48a4..eee00d2b 100644 --- a/tests/functional/resources/test_connection.py +++ b/tests/functional/resources/test_connection.py @@ -130,7 +130,7 @@ def test_empty_decl_derived_user(resource_funcs, res_opts, github_profile, for_r assert resource_funcs["github.discover"](opts) == ["user1/repo-a", "user1/user-repo"] with for_resource("user1/user-repo"): grains = resource_funcs["github.grains"]() - assert grains["github"]["owner"] == "user1" + assert grains["owner"] == "user1" def test_empty_decl_none_value(resource_funcs, res_opts): @@ -205,19 +205,30 @@ def test_grains(connection, for_resource): with for_resource("testorg/repo-a"): grains = connection["github.grains"]() assert grains == { - "github": { - "id": "testorg/repo-a", - "owner": "testorg", - "repo": "repo-a", - "full_name": "testorg/repo-a", - "private": False, - "fork": False, - "default_branch": "main", - "language": "Python", - "html_url": "https://github.example.com/testorg/repo-a", - "description": "Test repo repo-a", - "homepage": None, - } + "id": "testorg/repo-a", + "owner": "testorg", + "repo": "repo-a", + "full_name": "testorg/repo-a", + "private": False, + "visibility": "public", + "fork": False, + "archived": False, + "disabled": False, + "is_template": False, + "default_branch": "main", + "language": "Python", + "license": None, + "topics": [], + "custom_properties": {}, + "html_url": "https://github.example.com/testorg/repo-a", + "clone_url": "https://github.example.com/testorg/repo-a.git", + "ssh_url": "git@github.example.com:testorg/repo-a.git", + "description": "Test repo repo-a", + "homepage": None, + "created_at": "2026-10-11T13:37:42+00:00", + "has_discussions": False, + "has_pages": False, + "has_projects": True, } @@ -237,8 +248,8 @@ def test_grains_cold_context(res_opts, utils_loader, for_resource): funcs["github.init"](res_opts) with for_resource("user1/repo-a"): grains = funcs["github.grains"]() - assert grains["github"]["description"] == "User-owned repo-a" - assert grains["github"]["owner"] == "user1" + assert grains["description"] == "User-owned repo-a" + assert grains["owner"] == "user1" def test_grains_identity_routing(connection, for_resource): @@ -250,18 +261,18 @@ def test_grains_identity_routing(connection, for_resource): org_grains = connection["github.grains"]() with for_resource("user1/repo-a"): user_grains = connection["github.grains"]() - assert org_grains["github"]["description"] == "Test repo repo-a" - assert user_grains["github"]["description"] == "User-owned repo-a" + assert org_grains["description"] == "Test repo repo-a" + assert user_grains["description"] == "User-owned repo-a" def test_grains_refresh(connection, for_resource, fake_org): with for_resource("testorg/repo-a"): - assert connection["github.grains"]()["github"]["description"] == "Test repo repo-a" + assert connection["github.grains"]()["description"] == "Test repo repo-a" fake_org.get_repo("repo-a").update(description="Changed behind the cache") # Plain grains() serves from the warmed cache - assert connection["github.grains"]()["github"]["description"] == "Test repo repo-a" + assert connection["github.grains"]()["description"] == "Test repo repo-a" refreshed = connection["github.grains_refresh"]() - assert refreshed["github"]["description"] == "Changed behind the cache" + assert refreshed["description"] == "Changed behind the cache" def test_ping(connection, for_resource): diff --git a/tests/functional/resources/test_grains_files.py b/tests/functional/resources/test_grains_files.py new file mode 100644 index 00000000..07e579c1 --- /dev/null +++ b/tests/functional/resources/test_grains_files.py @@ -0,0 +1,191 @@ +""" +Tests for custom grains sourced from in-repo YAML/JSON files +(the ``grains_files`` resource declaration key). +""" + +import copy +import logging + +import pytest +import salt.loader +from salt.exceptions import CommandExecutionError + +GRAINS_FILES = { + "copier": { + "path": ".copier-answers.yml", + "exclude": ["_*"], + }, + "data": { + "path": [".github/metadata.json", ".github/other-metadata.json"], + "include": [ + "team", + "org:config", + {"service_tier": "service:tier"}, + ], + }, + # The special `_` namespace merges into the root grains + "_": { + "path": ".github/other-metadata.json", + "include": ["extra", "owner", {"meta_team": "team"}], + }, +} + + +@pytest.fixture(scope="module") +def resource_decl(): + return { + "profile": "github", + "grains_files": GRAINS_FILES, + "owners": { + "testorg": {}, + "user1": { + "grains_files": { + "copier": { + "path": ".copier-answers.yml", + # Mapping shorthand for rename-only includes + "include": {"template_version": "_commit"}, + }, + }, + }, + }, + } + + +@pytest.fixture +def connection(resource_funcs, res_opts): + resource_funcs["github.init"](res_opts) + return resource_funcs + + +def test_grains_custom_namespaces(connection, for_resource): + with for_resource("testorg/repo-a"): + grains = connection["github.grains"]() + # Identity grains are unaffected + assert grains["id"] == "testorg/repo-a" + assert grains["default_branch"] == "main" + # copier: everything but the excluded underscore keys + assert grains["copier"] == {"project_name": "Repo A", "team": "platform"} + # data: two files shallow-merged (earlier wins), include patterns + # preserving nesting, rename exposed as flat key + assert grains["data"] == { + "team": "json-team", + "org": {"config": {"visibility": "public"}}, + "service_tier": "gold", + } + # `_` namespace: merged into the root grains... + assert grains["extra"] is True + assert grains["meta_team"] == "other-team" + # ...but never overwriting builtin identity grains + assert grains["owner"] == "testorg" + + +def test_grains_all_files_missing(connection, for_resource): + # repo-b carries none of the declared files + with for_resource("testorg/repo-b"): + grains = connection["github.grains"]() + assert grains["repo"] == "repo-b" + assert "copier" not in grains + assert "data" not in grains + + +def test_grains_per_owner_override_replaces(connection, for_resource): + """ + A per-owner ``grains_files`` fully replaces the declaration-level + one: the override's include/rename applies instead of the default + exclude, and the declaration's other namespaces don't apply. + """ + with for_resource("user1/user-repo"): + grains = connection["github.grains"]() + assert grains["copier"] == {"template_version": "u9.9.9"} + assert "data" not in grains + assert "extra" not in grains + + +def test_grains_unparsable_file_skipped(connection, for_resource, fake_org, caplog): + fake_org.get_repo("repo-a").get_file(".copier-answers.yml").update(content="a: [unclosed") + with caplog.at_level(logging.WARNING): + with for_resource("testorg/repo-a"): + grains = connection["github.grains"]() + assert "Failed parsing" in caplog.text + assert "copier" not in grains + # Identity grains and the other namespace survive + assert grains["repo"] == "repo-a" + assert grains["data"]["team"] == "json-team" + + +def test_grains_non_mapping_file_skipped(connection, for_resource, fake_org, caplog): + fake_org.get_repo("repo-a").get_file(".copier-answers.yml").update(content="- a\n- b") + with caplog.at_level(logging.WARNING): + with for_resource("testorg/repo-a"): + grains = connection["github.grains"]() + assert "does not contain a mapping" in caplog.text + assert "copier" not in grains + assert grains["repo"] == "repo-a" + + +def test_init_invalid_grains_files(resource_funcs, res_opts): + opts = copy.deepcopy(res_opts) + opts["pillar"]["resources"]["github"]["grains_files"] = {"owner": {"path": "f"}} + with pytest.raises(CommandExecutionError, match="shadows a builtin"): + resource_funcs["github.init"](opts) + + +def test_init_invalid_per_owner_grains_files(resource_funcs, res_opts): + opts = copy.deepcopy(res_opts) + opts["pillar"]["resources"]["github"]["owners"]["user1"]["grains_files"] = {"ns": {}} + with pytest.raises(CommandExecutionError, match="'path' is required"): + resource_funcs["github.init"](opts) + + +def test_grains_file_fetches_cached(connection, for_resource, fake_github): + with for_resource("testorg/repo-a"): + connection["github.grains"]() + # 3 distinct paths: the `_` namespace reuses the `data` + # namespace's cached fetch of other-metadata.json + assert len(fake_github.filter_requests(path="*/contents/*")) == 3 + connection["github.grains"]() + # Served from the connection cache, no further requests + assert len(fake_github.filter_requests(path="*/contents/*")) == 3 + + +def test_grains_negative_fetches_cached(connection, for_resource, fake_github): + with for_resource("testorg/repo-b"): + connection["github.grains"]() + # All three declared paths are missing and were tried once + assert len(fake_github.filter_requests(path="*/contents/*")) == 3 + connection["github.grains"]() + assert len(fake_github.filter_requests(path="*/contents/*")) == 3 + + +def test_grains_refresh_refetches_files(connection, for_resource, fake_org): + with for_resource("testorg/repo-a"): + assert connection["github.grains"]()["copier"]["team"] == "platform" + fake_org.get_repo("repo-a").get_file(".copier-answers.yml").update( + content="team: replatformed" + ) + # Plain grains() serves from the warmed cache + assert connection["github.grains"]()["copier"]["team"] == "platform" + refreshed = connection["github.grains_refresh"]() + assert refreshed["copier"] == {"team": "replatformed"} + + +def test_grains_per_owner_empty_disables(res_opts, utils_loader, for_resource): + """ + A per-owner ``grains_files: {}`` is a full override as well, + disabling custom grains for that owner. + """ + opts = copy.deepcopy(res_opts) + opts["pillar"]["resources"]["github"]["owners"]["testorg"] = {"grains_files": {}} + cold_context = {} + funcs = salt.loader.resource( + opts, + utils=utils_loader, + context=cold_context, + loaded_base_name=f"{__name__}.empty", + ) + funcs["github.init"](opts) + with for_resource("testorg/repo-a"): + grains = funcs["github.grains"]() + assert "copier" not in grains + assert "data" not in grains + assert grains["repo"] == "repo-a" diff --git a/tests/functional/resources/test_multi_profile.py b/tests/functional/resources/test_multi_profile.py index 04bb770d..e16580d2 100644 --- a/tests/functional/resources/test_multi_profile.py +++ b/tests/functional/resources/test_multi_profile.py @@ -95,17 +95,19 @@ def test_discover_uses_owner_profile(connection, res_opts): def test_grains_per_owner(connection, for_resource): with for_resource("otherorg/other-repo"): grains = connection["github.grains"]() - assert grains["github"]["full_name"] == "otherorg/other-repo" - assert grains["github"]["owner"] == "otherorg" + assert grains["full_name"] == "otherorg/other-repo" + assert grains["owner"] == "otherorg" def test_exec_resolves_per_owner_profile(res_modules, for_resource, loader_context): with for_resource("otherorg/other-repo"): info = res_modules["repo.info"]() assert info["owner"] == "otherorg" - # The connection was built from the github_other profile: its cache - # key carries that profile's owner. - conn_keys = [ - key for key in loader_context if isinstance(key, str) and key.startswith("github.") - ] - assert any(":otherorg:" in key for key in conn_keys) + assert "github.profile.github_other" in loader_context + assert loader_context["github.profile.github_other"].owner == "otherorg" + ident = loader_context["github.profile.github_other"].ident + assert f"github.conn.{ident}" in loader_context + assert ( + loader_context[f"github.conn.{ident}"].client + is loader_context["github.profile.github_other"].client + ) diff --git a/tests/functional/resources/test_persistent_cache.py b/tests/functional/resources/test_persistent_cache.py new file mode 100644 index 00000000..715d1e90 --- /dev/null +++ b/tests/functional/resources/test_persistent_cache.py @@ -0,0 +1,191 @@ +""" +Tests for the persistent connection cache (the ``cache`` profile +parameter, enabled by default). + +The shared test profile opts out for test isolation; this module +removes the key again, exercising the on-by-default behavior, and +flushes the cache between tests itself. + +Fresh loader contexts simulate separate processes (as with +``salt-call --local``, where in-memory caches never survive): only the +persistent cache in the minion's ``cachedir`` carries over. +""" + +import copy +import os +import shutil + +import pytest +import salt.loader + +from saltext.github.utils.gh import profile as gh_profile +from saltext.github.utils.gh.cache import CACHE_ROOT + +GRAINS_FILES = { + "copier": { + "path": ".copier-answers.yml", + "exclude": ["_*"], + }, +} + + +@pytest.fixture(scope="module") +def resource_decl(): + return {"profile": "github", "grains_files": GRAINS_FILES} + + +@pytest.fixture(scope="module") +def res_opts(minion_opts, resource_decl): + opts = copy.deepcopy(minion_opts) + # Drop the shared test profile's opt-out to exercise the + # on-by-default behavior + opts["github"].pop("cache", None) + opts["pillar"] = {"resources": {"github": resource_decl}} + return opts + + +@pytest.fixture(autouse=True) +def _flush_persistent_cache(res_opts): + """ + The persistent cache outlives the per-test fake state reset by + design — isolate the tests. + """ + try: + yield + finally: + shutil.rmtree(os.path.join(res_opts["cachedir"], CACHE_ROOT), ignore_errors=True) + + +@pytest.fixture +def cold_funcs(res_opts, utils_loader): + """ + Factory for resource loaders with a fresh context, simulating a new + process. + """ + counter = iter(range(100)) + + def _build(opts=None): + funcs = salt.loader.resource( + opts or res_opts, + utils=utils_loader, + context={}, + loaded_base_name=f"{__name__}.cold{next(counter)}", + ) + funcs["github.init"](opts or res_opts) + return funcs + + return _build + + +def _contents_requests(fake_github): + return len(fake_github.filter_requests(path="*/contents/*")) + + +def test_file_fetches_persist_across_processes(cold_funcs, for_resource, fake_github): + with for_resource("testorg/repo-a"): + grains = cold_funcs()["github.grains"]() + assert grains["copier"]["team"] == "platform" + assert _contents_requests(fake_github) == 1 + with for_resource("testorg/repo-a"): + grains = cold_funcs()["github.grains"]() + assert grains["copier"]["team"] == "platform" + # Served from the persistent cache: no new contents request + assert _contents_requests(fake_github) == 1 + + +def test_negative_fetches_persist_across_processes(cold_funcs, for_resource, fake_github): + # repo-b does not carry the declared file + with for_resource("testorg/repo-b"): + grains = cold_funcs()["github.grains"]() + assert "copier" not in grains + assert _contents_requests(fake_github) == 1 + with for_resource("testorg/repo-b"): + cold_funcs()["github.grains"]() + assert _contents_requests(fake_github) == 1 + + +def test_push_invalidates(cold_funcs, for_resource, fake_github, fake_org): + with for_resource("testorg/repo-a"): + cold_funcs()["github.grains"]() + repo = fake_org.get_repo("repo-a") + repo.get_file(".copier-answers.yml").update(content="team: pushed") + repo.update(pushed_at="2026-10-15T00:00:00Z") + with for_resource("testorg/repo-a"): + grains = cold_funcs()["github.grains"]() + assert grains["copier"] == {"team": "pushed"} + assert _contents_requests(fake_github) == 2 + + +def test_stale_without_push(cold_funcs, for_resource, fake_github, fake_org): + """ + Cache entries are valid until the repository's ``pushed_at`` + changes — content changes without a push (impossible on real + GitHub) are not detected. + """ + with for_resource("testorg/repo-a"): + cold_funcs()["github.grains"]() + fake_org.get_repo("repo-a").get_file(".copier-answers.yml").update(content="team: changed") + with for_resource("testorg/repo-a"): + grains = cold_funcs()["github.grains"]() + assert grains["copier"]["team"] == "platform" + assert _contents_requests(fake_github) == 1 + + +def test_grains_refresh_bypasses_and_rewrites(cold_funcs, for_resource, fake_github, fake_org): + with for_resource("testorg/repo-a"): + cold_funcs()["github.grains"]() + fake_org.get_repo("repo-a").get_file(".copier-answers.yml").update(content="team: changed") + with for_resource("testorg/repo-a"): + refreshed = cold_funcs()["github.grains_refresh"]() + assert refreshed["copier"]["team"] == "changed" + fetched = _contents_requests(fake_github) + assert fetched == 2 + # The refresh rewrote the persistent entry: a new process serves it + with for_resource("testorg/repo-a"): + grains = cold_funcs()["github.grains"]() + assert grains["copier"]["team"] == "changed" + assert _contents_requests(fake_github) == fetched + + +def test_owner_type_persists_across_processes(cold_funcs, res_opts, fake_github): + cold_funcs()["github.discover"](res_opts) + assert len(fake_github.filter_requests(path="/users/testorg")) == 1 + cold_funcs()["github.discover"](res_opts) + # The account type lookup is served from the persistent cache + assert len(fake_github.filter_requests(path="/users/testorg")) == 1 + + +def test_auth_login_persists_across_processes(cold_funcs, res_opts, github_profile, fake_github): + """ + With an ownerless profile, the default owner is the token's user, + whose login lookup (GET /user) persists across processes. + """ + opts = copy.deepcopy(res_opts) + opts["github"] = {"token": github_profile["token"], "api_url": github_profile["api_url"]} + assert cold_funcs(opts)["github.discover"](opts) == ["user1/repo-a", "user1/user-repo"] + assert len(fake_github.filter_requests(path="/user")) == 1 + assert cold_funcs(opts)["github.discover"](opts) == ["user1/repo-a", "user1/user-repo"] + assert len(fake_github.filter_requests(path="/user")) == 1 + + +def test_identity_persists_for_classic_invocations(res_opts, fake_github): + """ + The cache is a profile-level feature: connections resolved outside + the resource subsystem persist token/account identities as well. + """ + conn = gh_profile.get_conn("github", opts=res_opts, context={}) + assert conn.owner_type("user1") == "User" + assert len(fake_github.filter_requests(path="/users/user1")) == 1 + conn = gh_profile.get_conn("github", opts=res_opts, context={}) + assert conn.owner_type("user1") == "User" + assert len(fake_github.filter_requests(path="/users/user1")) == 1 + + +def test_cache_disabled_via_profile(cold_funcs, res_opts, for_resource, fake_github): + opts = copy.deepcopy(res_opts) + opts["github"]["cache"] = False + with for_resource("testorg/repo-a"): + cold_funcs(opts)["github.grains"]() + cold_funcs(opts)["github.grains"]() + # Without the persistent cache, each fresh process refetches + assert _contents_requests(fake_github) == 2 diff --git a/tests/support/github/__init__.py b/tests/support/github/__init__.py index 2a00dc2f..b8a1f5a5 100644 --- a/tests/support/github/__init__.py +++ b/tests/support/github/__init__.py @@ -43,6 +43,7 @@ from tests.support.github.entities import Pages from tests.support.github.entities import PullRequest from tests.support.github.entities import Repo +from tests.support.github.entities import RepoFile from tests.support.github.entities import RepoId from tests.support.github.entities import Ruleset from tests.support.github.entities import Secret @@ -50,6 +51,7 @@ from tests.support.github.entities import TeamMember from tests.support.github.entities import User from tests.support.github.routes import ActionsRoutes +from tests.support.github.routes import ContentsRoutes from tests.support.github.routes import DeployKeyRoutes from tests.support.github.routes import EnvironmentRoutes from tests.support.github.routes import IssueRoutes @@ -89,6 +91,7 @@ "Pages", "PullRequest", "Repo", + "RepoFile", "RepoId", "Request", "Ruleset", @@ -112,6 +115,7 @@ class FakeGitHubAPI( # pylint: disable=too-many-ancestors LabelRoutes, PagesRoutes, MetaRoutes, + ContentsRoutes, ): """ Dict-backed implementation of the consumed GitHub API surface. diff --git a/tests/support/github/entities.py b/tests/support/github/entities.py index 427d0475..2b2c3f97 100644 --- a/tests/support/github/entities.py +++ b/tests/support/github/entities.py @@ -14,6 +14,7 @@ # with the GlobalState they are bound to, while the state stores entity data. # pyright: reportImportCycles=false +import base64 import zlib from collections.abc import Callable from collections.abc import ItemsView @@ -489,9 +490,20 @@ class RepoData(EntityData): # pylint: disable=too-many-instance-attributes private: bool = False fork: bool = False archived: bool = False + disabled: bool = False + is_template: bool = False language: str = "Python" + # SPDX identifier, rendered as a nested license object in the payload + license: str | None = None + topics: list[str] = field(default_factory=list) + custom_properties: dict[str, Any] = field(default_factory=dict) default_branch: str = "main" + created_at: str = CREATED_AT + updated_at: str = CREATED_AT + pushed_at: str = CREATED_AT + has_discussions: bool = False has_issues: bool = True + has_projects: bool = True has_wiki: bool = True has_downloads: bool = True # Managed via the Actions permissions endpoints, not part of the repo payload @@ -716,6 +728,28 @@ def remove_label(self, name: str, *, exc=NotFound) -> None: raise exc(f"Label {name=} does not exist for repo {self.data.repo_id}") del self.gh.labels[self.data.repo_id][name] + @property + def files(self) -> "EntityView[str, RepoFile]": + return EntityView( + { + path: RepoFile(self.gh, data) + for path, data in self.gh.files[self.data.repo_id].items() + } + ) + + def add_file(self, path: str, content: str = "") -> "RepoFile": + return self.gh.add(RepoFileData(path, self.data.repo_id, content=content)) + + def get_file(self, path: str, *, exc=NotFound) -> "RepoFile": + if path not in self.gh.files[self.data.repo_id]: + raise exc(f"File {path=} does not exist in repo {self.data.repo_id}") + return RepoFile(self.gh, self.gh.files[self.data.repo_id][path]) + + def remove_file(self, path: str, *, exc=NotFound) -> None: + if path not in self.gh.files[self.data.repo_id]: + raise exc(f"File {path=} does not exist in repo {self.data.repo_id}") + del self.gh.files[self.data.repo_id][path] + @property def pages(self) -> "Pages | None": if self.data.repo_id not in self.gh.pages: @@ -745,6 +779,11 @@ def payload(self, /, **ovrr): "owner": {"login": self.data.owner}, "url": f"{self.gh.base_url}/repos/{self.data.full_name}", "html_url": f"https://github.example.com/{self.data.full_name}", + "clone_url": f"https://github.example.com/{self.data.full_name}.git", + "ssh_url": f"git@github.example.com:{self.data.full_name}.git", + "visibility": "private" if self.data.private else "public", + "license": {"spdx_id": self.data.license} if self.data.license else None, + "has_pages": self.data.repo_id in self.gh.pages, "size": 100, "stargazers_count": 1, "watchers_count": 1, @@ -1261,6 +1300,36 @@ def payload(self, /, **ovrr): ) +@dataclass +class RepoFileData(EntityData): + path: str + repo: RepoId + _: KW_ONLY + content: str = "" + + def __post_init__(self): + # File paths are only unique per repo, include it in the ID. + self.id = zlib.crc32(f"{type(self).__name__}/{self.repo}/{self.path}".encode()) + + +@dataclass +class RepoFile(Entity): + data: RepoFileData + + def payload(self, /, **ovrr): + content = self.data.content.encode() + return { + "type": "file", + "name": self.data.path.rsplit("/", maxsplit=1)[-1], + "path": self.data.path, + "sha": f"{self.data.id:x}", + "size": len(content), + "encoding": "base64", + "content": base64.b64encode(content).decode(), + "url": f"{self.gh.base_url}/repos/{self.data.repo}/contents/{self.data.path}", + } | ovrr + + @dataclass class PagesData(EntityData): repo: RepoId diff --git a/tests/support/github/routes/__init__.py b/tests/support/github/routes/__init__.py index 28d00e8c..b5a9355f 100644 --- a/tests/support/github/routes/__init__.py +++ b/tests/support/github/routes/__init__.py @@ -3,6 +3,7 @@ """ from tests.support.github.routes.actions import ActionsRoutes +from tests.support.github.routes.contents import ContentsRoutes from tests.support.github.routes.deploy_keys import DeployKeyRoutes from tests.support.github.routes.environments import EnvironmentRoutes from tests.support.github.routes.issues import IssueRoutes @@ -17,6 +18,7 @@ __all__ = [ "ActionsRoutes", + "ContentsRoutes", "DeployKeyRoutes", "EnvironmentRoutes", "IssueRoutes", diff --git a/tests/support/github/routes/contents.py b/tests/support/github/routes/contents.py new file mode 100644 index 00000000..b0edaac7 --- /dev/null +++ b/tests/support/github/routes/contents.py @@ -0,0 +1,15 @@ +""" +Routes: repository contents. +""" + +from tests.support.github.base import APIBase +from tests.support.github.routing import GET + + +class ContentsRoutes(APIBase): + # Full regex: file paths contain slashes, which the `` + # shorthand expansion does not cover. + @GET(r"/repos/(?P[^/]+)/(?P[^/]+)/contents/(?P.+)") + def _get_content(self, match, _query, _body): + repo = self._resolve_repo(match) + return 200, repo.get_file(match["path"]).payload() diff --git a/tests/support/github/state.py b/tests/support/github/state.py index b024f6ab..3613cfeb 100644 --- a/tests/support/github/state.py +++ b/tests/support/github/state.py @@ -36,6 +36,8 @@ from tests.support.github.entities import PullRequestData from tests.support.github.entities import Repo from tests.support.github.entities import RepoData +from tests.support.github.entities import RepoFile +from tests.support.github.entities import RepoFileData from tests.support.github.entities import RepoId from tests.support.github.entities import RepoRulesetData from tests.support.github.entities import Ruleset @@ -78,6 +80,7 @@ class GlobalState: # pylint: disable=too-many-instance-attributes default_factory=lambda: defaultdict(dict) ) labels: dict[RepoId, dict[str, LabelData]] = field(default_factory=lambda: defaultdict(dict)) + files: dict[RepoId, dict[str, RepoFileData]] = field(default_factory=lambda: defaultdict(dict)) pages: dict[RepoId, PagesData] = field(default_factory=dict) def __post_init__(self): @@ -164,6 +167,7 @@ def remove_repo(self, repo_id: RepoId | Sequence[str] | str, *, exc=NotFound) -> self.secrets, self.environments, self.labels, + self.files, ): store.pop(resolved, None) self.pages.pop(resolved, None) @@ -224,5 +228,6 @@ def replace(self, old: EntityData | Entity, new: EntityData | Entity): SecretData: (Secret, "secrets", lambda d: d.repo, lambda d: d.name, "repos"), EnvironmentData: (Environment, "environments", lambda d: d.repo, lambda d: d.name, "repos"), LabelData: (Label, "labels", lambda d: d.repo, lambda d: d.name, "repos"), + RepoFileData: (RepoFile, "files", lambda d: d.repo, lambda d: d.path, "repos"), PagesData: (Pages, "pages", lambda d: d.repo, None, "repos"), } diff --git a/tests/unit/utils/test_cache.py b/tests/unit/utils/test_cache.py new file mode 100644 index 00000000..b19ce493 --- /dev/null +++ b/tests/unit/utils/test_cache.py @@ -0,0 +1,112 @@ +import pytest +import salt.cache + +from saltext.github.utils.gh import cache as gh_cache + +PUSHED = "2026-10-14T12:00:00+00:00" + + +class FakeSaltCache: + def __init__(self): + self.data = {} + self.fail = False + + def _check(self): + if self.fail: + raise OSError("disk on fire") + + def fetch(self, bank, key): + self._check() + return self.data.get((bank, key), {}) + + def store(self, bank, key, value): + self._check() + self.data[(bank, key)] = value + + def flush(self, bank, key=None): + self._check() + self.data.pop((bank, key), None) + + +@pytest.fixture +def backend(monkeypatch): + fake = FakeSaltCache() + monkeypatch.setattr(salt.cache, "factory", lambda opts: fake) + return fake + + +@pytest.fixture +def store(backend): # pylint: disable=unused-argument + return gh_cache.PersistentCache({}, "ident123") + + +def test_meta_roundtrip(store): + assert store.get_meta("auth_login") is gh_cache.MISS + store.set_meta("auth_login", "user1") + assert store.get_meta("auth_login") == "user1" + + +def test_meta_none_value_is_not_a_miss(store): + store.set_meta("something", None) + assert store.get_meta("something") is None + + +def test_file_roundtrip(store): + assert store.get_file("Org", "Repo", "a.yml", PUSHED) is gh_cache.MISS + store.set_file("Org", "Repo", "a.yml", "content", PUSHED) + # Owner/repo lookups are case-insensitive, paths are not + assert store.get_file("org", "repo", "a.yml", PUSHED) == "content" + assert store.get_file("org", "repo", "A.yml", PUSHED) is gh_cache.MISS + + +def test_file_negative_content_is_not_a_miss(store): + store.set_file("org", "repo", "missing.yml", None, PUSHED) + assert store.get_file("org", "repo", "missing.yml", PUSHED) is None + + +def test_file_pushed_at_mismatch_is_a_miss(store): + store.set_file("org", "repo", "a.yml", "content", PUSHED) + assert store.get_file("org", "repo", "a.yml", "2026-10-15T00:00:00+00:00") is gh_cache.MISS + + +def test_file_new_pushed_at_discards_stale_siblings(store): + store.set_file("org", "repo", "a.yml", "old-a", PUSHED) + newer = "2026-10-15T00:00:00+00:00" + store.set_file("org", "repo", "b.yml", "new-b", newer) + assert store.get_file("org", "repo", "b.yml", newer) == "new-b" + # a.yml was stored for the old pushed_at and must not survive + assert store.get_file("org", "repo", "a.yml", newer) is gh_cache.MISS + assert store.get_file("org", "repo", "a.yml", PUSHED) is gh_cache.MISS + + +def test_file_unknown_pushed_at_disables(store, backend): + assert store.get_file("org", "repo", "a.yml", None) is gh_cache.MISS + store.set_file("org", "repo", "a.yml", "content", None) + assert not backend.data + + +def test_flush_repo(store): + store.set_file("org", "repo", "a.yml", "content", PUSHED) + store.set_file("org", "other", "a.yml", "other", PUSHED) + store.flush_repo("ORG", "Repo") + assert store.get_file("org", "repo", "a.yml", PUSHED) is gh_cache.MISS + assert store.get_file("org", "other", "a.yml", PUSHED) == "other" + + +def test_ident_namespacing(backend): # pylint: disable=unused-argument + one = gh_cache.PersistentCache({}, "ident1") + two = gh_cache.PersistentCache({}, "ident2") + one.set_meta("auth_login", "user1") + assert two.get_meta("auth_login") is gh_cache.MISS + + +def test_backend_errors_degrade_to_misses(store, backend, caplog): + store.set_file("org", "repo", "a.yml", "content", PUSHED) + backend.fail = True + with caplog.at_level("WARNING"): + assert store.get_file("org", "repo", "a.yml", PUSHED) is gh_cache.MISS + assert store.get_meta("auth_login") is gh_cache.MISS + store.set_meta("auth_login", "user1") # must not raise + store.set_file("org", "repo", "b.yml", "x", PUSHED) # must not raise + store.flush_repo("org", "repo") # must not raise + assert "persistent GitHub cache" in caplog.text diff --git a/tests/unit/utils/test_grains_files.py b/tests/unit/utils/test_grains_files.py new file mode 100644 index 00000000..d3d1a798 --- /dev/null +++ b/tests/unit/utils/test_grains_files.py @@ -0,0 +1,411 @@ +import logging + +import pytest +from salt.exceptions import CommandExecutionError + +from saltext.github.utils.gh import grains_files as gh_grains_files +from saltext.github.utils.gh import repos as gh_repos +from saltext.github.utils.gh.exceptions import GitHubApiError + +log = logging.getLogger(__name__) + + +# ------------------------------------------------------------------------- +# validate +# ------------------------------------------------------------------------- + + +def _is_empty_dict(res): + return isinstance(res, dict) and not res + + +def test_validate_unset(): + assert _is_empty_dict(gh_grains_files.validate(None)) + assert _is_empty_dict(gh_grains_files.validate({})) + + +def test_validate_normalization(): + spec = { + "copier": { + "path": ".copier-answers.yml", + "exclude": ["_*"], + }, + "data": { + "path": [".github/metadata.json", ".github/other-metadata.json"], + "include": [ + "team", + "org:config", + {"service_tier": "service:tier"}, + ], + }, + } + assert gh_grains_files.validate(spec) == { + "copier": { + "path": [".copier-answers.yml"], + "include": None, + "exclude": ["_*"], + }, + "data": { + "path": [".github/metadata.json", ".github/other-metadata.json"], + "include": ["team", "org:config", ("service_tier", "service:tier")], + "exclude": [], + }, + } + + +def test_validate_include_mapping_shorthand(): + spec = gh_grains_files.validate( + {"ns": {"path": "f", "include": {"a": "b:c", "d": "e", "f": None, "g*": ""}}} + ) + assert spec["ns"]["include"] == [("a", "b:c"), ("d", "e"), "f", "g*"] + + +def test_validate_include_str(): + spec = gh_grains_files.validate({"ns": {"path": "f", "include": "foo*"}}) + assert spec["ns"]["include"] == ["foo*"] + + +def test_validate_accepts_root_namespace(): + assert gh_grains_files.validate({"_": {"path": "f"}}) == { + "_": {"path": ["f"], "include": None, "exclude": []} + } + + +@pytest.mark.parametrize( + "spec,err", + [ + (["foo"], "must be a mapping of namespace"), + ({"owner": {"path": "f"}}, "shadows a builtin resource grain"), + ({42: {"path": "f"}}, "must be non-empty strings"), + ({"": {"path": "f"}}, "must be non-empty strings"), + ({"ns": ["f"]}, "specification must be a mapping"), + ({"ns": {}}, "'path' is required"), + ({"ns": {"path": []}}, "'path' is required"), + ({"ns": {"path": ["f", 2]}}, "'path' is required"), + ({"ns": {"path": "f", "include": [{"a": "b", "c": "d"}]}}, "single-entry"), + ({"ns": {"path": "f", "include": [{"a": 2}]}}, "single-entry"), + ({"ns": {"path": "f", "include": [2]}}, "'include' entries must be"), + ({"ns": {"path": "f", "exclude": [2]}}, "'exclude' must be a list"), + ({"ns": {"path": "f", "includes": []}}, "unknown key"), + ], +) +def test_validate_errors(spec, err): + with pytest.raises(CommandExecutionError, match=err): + gh_grains_files.validate(spec) + + +# ------------------------------------------------------------------------- +# filter_content +# ------------------------------------------------------------------------- + + +@pytest.fixture +def data(): + return { + "team": "platform", + "tier": 2, + "_commit": "v1.2.3", + "_src_path": "gh:org/tpl", + "org": { + "config": {"visibility": "public"}, + "other": True, + }, + "service": {"tier": "gold", "name": "svc"}, + } + + +def test_filter_no_filters_deep_copies(data): + res = gh_grains_files.filter_content(data) + assert res == data + assert res is not data + res["org"]["config"]["visibility"] = "private" + assert data["org"]["config"]["visibility"] == "public" + + +def test_filter_include_exact(data): + assert gh_grains_files.filter_content(data, include=["team"]) == {"team": "platform"} + + +def test_filter_include_wildcard(data): + assert gh_grains_files.filter_content(data, include=["t*"]) == { + "team": "platform", + "tier": 2, + } + + +def test_filter_include_nested_path_preserves_nesting(data): + assert gh_grains_files.filter_content(data, include=["org:config"]) == { + "org": {"config": {"visibility": "public"}} + } + + +def test_filter_include_intermediate_key_includes_subtree(data): + assert gh_grains_files.filter_content(data, include=["org"]) == { + "org": {"config": {"visibility": "public"}, "other": True} + } + + +def test_filter_include_no_match(data): + assert _is_empty_dict(gh_grains_files.filter_content(data, include=["nothing"])) + + +def test_filter_exclude_only(data): + res = gh_grains_files.filter_content(data, exclude=["_*"]) + assert res == { + "team": "platform", + "tier": 2, + "org": {"config": {"visibility": "public"}, "other": True}, + "service": {"tier": "gold", "name": "svc"}, + } + + +def test_filter_exclude_inside_included_subtree(data): + assert gh_grains_files.filter_content(data, include=["org"], exclude=["org:other"]) == { + "org": {"config": {"visibility": "public"}} + } + + +def test_filter_exclude_empties_included_subtree(data): + # The explicitly included (now empty) mapping is kept + assert gh_grains_files.filter_content(data, include=["org"], exclude=["org:*"]) == {"org": {}} + + +def test_filter_exclude_at_depth(): + data = {"a": {"b": {"c": 1, "d": 2}}} + assert gh_grains_files.filter_content(data, exclude=["a:b:c"]) == {"a": {"b": {"d": 2}}} + + +def test_filter_wildcard_crosses_segments(data): + # fnmatch `*` is not segment-aware: `org*` also matches `org:other` + # as an exclude at any depth + assert gh_grains_files.filter_content(data, include=["service"], exclude=["service*name"]) == { + "service": {"tier": "gold"} + } + + +def test_filter_rename_scalar(data): + assert gh_grains_files.filter_content(data, include=[("version", "_commit")]) == { + "version": "v1.2.3" + } + + +def test_filter_rename_subtree(data): + res = gh_grains_files.filter_content(data, include=[("conf", "org:config")]) + assert res == {"conf": {"visibility": "public"}} + res["conf"]["visibility"] = "mutated" + assert data["org"]["config"]["visibility"] == "public" + + +def test_filter_rename_missing_source(data): + assert _is_empty_dict(gh_grains_files.filter_content(data, include=[("x", "does:not:exist")])) + + +def test_filter_rename_through_scalar(data): + assert _is_empty_dict(gh_grains_files.filter_content(data, include=[("x", "team:sub")])) + + +def test_filter_rename_immune_to_exclude(data): + res = gh_grains_files.filter_content( + data, include=[("version", "_commit")], exclude=["_*", "version"] + ) + assert res == {"version": "v1.2.3"} + + +def test_filter_rename_first_definition_wins(data): + res = gh_grains_files.filter_content( + data, include=[("version", "_commit"), ("version", "org")], exclude=["_*", "version"] + ) + assert res == {"version": "v1.2.3"} + + +def test_filter_rename_overrides_included_key(data): + res = gh_grains_files.filter_content(data, include=["team", ("team", "service:name")]) + assert res == {"team": "svc"} + + +def test_filter_rename_to_nested_key(data): + res = gh_grains_files.filter_content(data, include=[("nested:target", "service:name")]) + assert res == {"nested": {"target": "svc"}} + + +def test_filter_rename_source_is_literal(data): + # No fnmatch in rename sources: `*` is a literal key + assert _is_empty_dict(gh_grains_files.filter_content(data, include=[("x", "te*m")])) + + +def test_filter_earlier_rename_wins(data): + res = gh_grains_files.filter_content(data, include=[("x", "team"), ("x", "service:name")]) + assert res == {"x": "platform"} + + +def _normalized(include=None, exclude=None): + spec = gh_grains_files.validate({"ns": {"path": "f", "include": include, "exclude": exclude}}) + return spec["ns"] + + +def test_filter_with_normalized_rename_tuples(data): + # filter_content consumes validate() output (tuples, not dicts) + entry = _normalized(include=["team", {"version": "_commit"}]) + res = gh_grains_files.filter_content(data, include=entry["include"], exclude=entry["exclude"]) + assert res == {"team": "platform", "version": "v1.2.3"} + + +# ------------------------------------------------------------------------- +# merge_grains +# ------------------------------------------------------------------------- + + +def test_merge_grains_namespaces_and_root(caplog): + grains = { + "id": "o/r", + "owner": "o", + "owner2": "o2", + "owner3": "o3", + "nested_owner": {"should_not": {"be_replaced": True}}, + } + custom = { + "copier": {"a": 1}, + "_": {"team": "platform", "owner": "evil", "copier": "nope"}, + "nested:ns": {"success": True}, + "owner2": {"evil": True}, + "owner3:ns": {"evil": True}, + "nested_owner:should_not": {"be_replaced": False, "not_evil": True}, + } + with caplog.at_level(logging.WARNING): + res = gh_grains_files.merge_grains(grains, custom) + assert res is grains + # `_` keys land at the root; existing keys (builtin `owner`, earlier + # namespace `copier`) are never overwritten + assert grains == { + "id": "o/r", + "owner": "o", + "owner2": "o2", + "owner3": "o3", + "copier": {"a": 1}, + "team": "platform", + "nested": {"ns": {"success": True}}, + "nested_owner": {"should_not": {"be_replaced": True, "not_evil": True}}, + } + assert "'copier' shadows an existing grain" in caplog.text + assert "'owner' shadows an existing grain" in caplog.text + assert "'owner2' shadows an existing grain" in caplog.text + assert "'owner3:ns' shadows an existing grain" in caplog.text + assert "'nested_owner:should_not:be_replaced' shadows an existing grain" in caplog.text + + +def test_merge_grains_first_wins(): + grains = {"id": "o/r"} + custom = {"_": {"copier": "root-val"}, "copier": {"a": 1}} + gh_grains_files.merge_grains(grains, custom) + assert grains["copier"] == "root-val" + + +# ------------------------------------------------------------------------- +# load_mapping +# ------------------------------------------------------------------------- + + +def test_load_mapping_json(): + res = gh_grains_files.load_mapping('{"a": 1, "b": {"c": true}}', path="x") + assert res == {"a": 1, "b": {"c": True}} + + +def test_load_mapping_yaml_types_normalized(): + res = gh_grains_files.load_mapping( + "date: 2026-10-14\nflag: false\nempty: null\nnum: 2.5", path="x" + ) + assert res["date"] == "2026-10-14" + assert res["flag"] is False + assert res["empty"] is None + assert res["num"] == 2.5 + + +@pytest.mark.parametrize("text", ["- a\n- b", "just a scalar", ""]) +def test_load_mapping_non_mapping(text, caplog): + with caplog.at_level(logging.WARNING): + assert gh_grains_files.load_mapping(text, path="x") is None + assert "does not contain a mapping" in caplog.text + + +def test_load_mapping_syntax_error(caplog): + with caplog.at_level(logging.WARNING): + assert gh_grains_files.load_mapping("a: [unclosed", path="x") is None + assert "Failed parsing" in caplog.text + + +# ------------------------------------------------------------------------- +# collect (fetch stubbed out) +# ------------------------------------------------------------------------- + + +class FakeConn: + owner = "testorg" + + +@pytest.fixture +def files(monkeypatch): + contents = {} + + def fake_get_file_content( + conn, name, path, *, owner=None, ignore_cache=False # pylint: disable=unused-argument + ): + res = contents.get((owner, name, path)) + if isinstance(res, Exception): + raise res + return res + + monkeypatch.setattr(gh_repos, "get_file_content", fake_get_file_content) + return contents + + +def test_collect_merges_first_wins(files): + files[("testorg", "repo", "a.yml")] = "common: 1\nnested:\n a: 1\nonly_a: true" + files[("testorg", "repo", "b.yml")] = "common: 2\nnested:\n b: 2\nonly_b: true" + spec = gh_grains_files.validate({"ns": {"path": ["a.yml", "b.yml"]}}) + res = gh_grains_files.collect(FakeConn(), "repo", spec) + # Very shallow merge: only undefined keys are merged and nested mappings are kept + assert res == {"ns": {"common": 1, "nested": {"a": 1}, "only_a": True, "only_b": True}} + + +def test_collect_missing_files_skipped(files): + files[("testorg", "repo", "b.yml")] = "found: true" + spec = gh_grains_files.validate( + {"ns": {"path": ["missing.yml", "b.yml"]}, "empty": {"path": "nope.yml"}} + ) + res = gh_grains_files.collect(FakeConn(), "repo", spec) + assert res == {"ns": {"found": True}} + + +def test_collect_fetch_error_skips_file(files, caplog): + files[("testorg", "repo", "a.yml")] = GitHubApiError("500 boom", status=500) + files[("testorg", "repo", "b.yml")] = "found: true" + spec = gh_grains_files.validate({"ns": {"path": ["a.yml", "b.yml"]}}) + with caplog.at_level(logging.WARNING): + res = gh_grains_files.collect(FakeConn(), "repo", spec) + assert res == {"ns": {"found": True}} + assert "Failed fetching 'a.yml'" in caplog.text + + +def test_collect_unparsable_file_skipped(files, caplog): + files[("testorg", "repo", "a.yml")] = "- not\n- a\n- mapping" + spec = gh_grains_files.validate({"ns": {"path": "a.yml"}}) + with caplog.at_level(logging.WARNING): + assert _is_empty_dict(gh_grains_files.collect(FakeConn(), "repo", spec)) + + +def test_collect_empty_filter_result_omitted(files): + files[("testorg", "repo", "a.yml")] = "_commit: v1" + spec = gh_grains_files.validate({"ns": {"path": "a.yml", "exclude": ["_*"]}}) + assert _is_empty_dict(gh_grains_files.collect(FakeConn(), "repo", spec)) + + +def test_collect_applies_filters(files): + files[("testorg", "repo", ".copier-answers.yml")] = "_commit: v1\nproject: salt" + spec = gh_grains_files.validate( + {"copier": {"path": ".copier-answers.yml", "include": [{"template_version": "_commit"}]}} + ) + res = gh_grains_files.collect(FakeConn(), "repo", spec, owner="other") + assert _is_empty_dict(res) # seeded under testorg, fetched for owner "other" + files[("other", "repo", ".copier-answers.yml")] = "_commit: v2\nproject: salt" + res = gh_grains_files.collect(FakeConn(), "repo", spec, owner="other") + assert res == {"copier": {"template_version": "v2"}}