Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support for passing additional parameters from app repository to root repository using override file #184

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: support for passing additional parameters from app repository to root repository using override file #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions docs/commands/sync-apps.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,41 @@ config:
app-xy-test:
```

## Passing additional custom keys from App Config to Root Config repositories

```
Section to be discussed, proposal only.
```

If App Team wants to pass additional parameters to the root repository - it may be done using additional custom_values.yaml located in the app_folder. Keys in this file will be validated against whitelist located in the root repository.
If whitelist.yaml is missing - by default only key teamcode is allowed.

### Files used during process

**root_repo/whitelist.yaml**
```yaml
teamcode: null
Copy link
Member

Choose a reason for hiding this comment

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

why not a list of allowed keys?

allowed_custom_parameters: 
- teamcode
- keyallowed

keyallowed: null
```
**app_repo/app-xy-test/custom_values.yaml**
Copy link
Member

Choose a reason for hiding this comment

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

maybe custom_project_config.yaml, custom_namespace_config.yaml or custom_root_config.yaml gives a bit more context to what the file is for

```yaml
teamcode: team-xy
keydisallowed: security-breach
```
### Resulting file
**root_repo/apps/team-a.yaml**
```yaml
repository: https://github.com/company-deployments/team-1-app-config-repo.git # link to your apps root repository

# The applications that are synced by the `sync-app` command:
applications:
app-xy-production: # <- every entry corresponds to a directory in the apps root repository
app-xy-staging:
teamcode: team-xy
app-xy-test:
```


## Example

```bash
Expand Down
62 changes: 54 additions & 8 deletions gitopscli/commands/sync_apps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
import logging
import os
from dataclasses import dataclass
from typing import Any, Set, Tuple
from typing import Any, Set, Tuple, Dict
from gitopscli.git_api import GitApiConfig, GitRepo, GitRepoApiFactory
from gitopscli.io_api.yaml_util import merge_yaml_element, yaml_file_load
from gitopscli.io_api.yaml_util import YAMLException, merge_yaml_element, yaml_file_load, yaml_load
from gitopscli.gitops_exception import GitOpsException
from .command import Command


class SyncAppsCommand(Command):
@dataclass(frozen=True)
class Args(GitApiConfig):
Expand Down Expand Up @@ -50,18 +49,65 @@ def __sync_apps(team_config_git_repo: GitRepo, root_config_git_repo: GitRepo, gi
apps_from_other_repos,
found_apps_path,
) = __find_apps_config_from_repo(team_config_git_repo, root_config_git_repo)

if current_repo_apps == repo_apps:
logging.info("Root repository already up-to-date. I'm done here.")
return

# TODO to be discussed - how to proceed with changes here, as adding additional custom_values will invalidate this check.
# Based on the outcome - test test_sync_apps_already_up_to_date also needs to be modified.
# Options:
# - remove this check
# - add validation of customizationfile presence to __find_apps_config_from_repo
# - move and modify this check to validate actual changes (get the applications list from resulting yaml and compare with current one)
# if current_repo_apps == repo_apps:
# logging.info("Root repository already up-to-date. I'm done here.")
# return

__check_if_app_already_exists(repo_apps, apps_from_other_repos)

logging.info("Sync applications in root repository's %s.", apps_config_file_name)
merge_yaml_element(apps_config_file, found_apps_path, {repo_app: {} for repo_app in repo_apps})

merge_yaml_element(
apps_config_file,
found_apps_path,
{repo_app: __clean_repo_app(root_config_git_repo, team_config_git_repo, repo_app) for repo_app in repo_apps},
)
__commit_and_push(team_config_git_repo, root_config_git_repo, git_user, git_email, apps_config_file_name)


def __clean_yaml(values: Dict[str, Any], whitelist: Dict[str, Any], app_name: str) -> Any:

whitelisted_yml = {k:v for k,v in values.items() if k in whitelist.keys()}
missing = set(values).difference(whitelisted_yml)
logging.info("Keys removed from %s custom_values.yaml %s", app_name, missing)
return whitelisted_yml


def __clean_repo_app(root_config_git_repo: GitRepo, team_config_git_repo: GitRepo, app_name: str) -> Any:
app_spec_file = team_config_git_repo.get_full_file_path(f"{app_name}/custom_values.yaml")
app_spec_whitelist_file = root_config_git_repo.get_full_file_path(f"whitelist.yaml")
try:
app_config_content = yaml_file_load(app_spec_file)
except FileNotFoundError as ex:
logging.warning("no specific app settings file found for %s", app_name)
return {}
except YAMLException as yex:
logging.error("Unable to load %s/custom_values.yaml from app repository, please validate if this is a correct YAML file" , exc_info=yex)
return {}
#TODO: should sync fail or skip adding custom values (return {})
#TODO: which errors should be raised as GitOpsException
try:
whitelist_config_content = yaml_file_load(app_spec_whitelist_file)
except FileNotFoundError as ex:
logging.warning("no whitelist app settings file found in root repo, by default allowing teamcode only")
whitelist_yaml_string = """\
teamcode: null
"""
whitelist_config_content = yaml_load(whitelist_yaml_string)
except YAMLException as yex:
logging.error("Unable to load whitelist.yaml from root repository, please validate if this is a correct YAML file" , exc_info=yex)
return {}
#TODO: should sync fail, assume default whitelist or skip adding custom values (return {})
Copy link
Member

Choose a reason for hiding this comment

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

Taking into account that the sync-apps command is usually run by some pipeline not directly maintained by the individual teams, it might be a bit hard to figure out what went wrong (or even notice that it failed in the first place).

A compromise could be to create a YAML comment in the resulting root config file with the failure message:

applications:
  app-xy-staging:
    teamcode: team-xy
   # sync-apps error: 'keydisallowed' not allowed (custom_values.yaml, line 3)

This way you neither have to swallow the error nor break the sync.

return __clean_yaml(app_config_content, whitelist_config_content, app_name)


def __find_apps_config_from_repo(
team_config_git_repo: GitRepo, root_config_git_repo: GitRepo
) -> Tuple[str, str, Set[str], Set[str], str]:
Expand Down
90 changes: 46 additions & 44 deletions tests/commands/test_sync_apps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,52 +122,53 @@ def test_sync_apps_happy_flow(self):
call.GitRepo_root.commit("GIT_USER", "GIT_EMAIL", "author updated apps/team-non-prod.yaml"),
call.GitRepo_root.push(),
]
# TODO: To be discussed how to progress with this function, as adding custom_values.yaml changes the app behaviour.
# def test_sync_apps_already_up_to_date(self):
# self.yaml_file_load_mock.side_effect = lambda file_path: {
# "/tmp/root-config-repo/bootstrap/values.yaml": {
# "bootstrap": [{"name": "team-non-prod"}, {"name": "other-team-non-prod"}],
# },
# "/tmp/root-config-repo/apps/team-non-prod.yaml": {
# "repository": "https://team.config.repo.git",
# "applications": {"my-app": None}, # my-app already exists
# },
# "/tmp/root-config-repo/apps/other-team-non-prod.yaml": {
# "repository": "https://other-team.config.repo.git",
# "applications": {},
# },
# }[file_path]

def test_sync_apps_already_up_to_date(self):
self.yaml_file_load_mock.side_effect = lambda file_path: {
"/tmp/root-config-repo/bootstrap/values.yaml": {
"bootstrap": [{"name": "team-non-prod"}, {"name": "other-team-non-prod"}],
},
"/tmp/root-config-repo/apps/team-non-prod.yaml": {
"repository": "https://team.config.repo.git",
"applications": {"my-app": None}, # my-app already exists
},
"/tmp/root-config-repo/apps/other-team-non-prod.yaml": {
"repository": "https://other-team.config.repo.git",
"applications": {},
},
}[file_path]
# SyncAppsCommand(ARGS).execute()
# assert self.mock_manager.method_calls == [
# call.GitRepoApiFactory.create(ARGS, "TEAM_ORGA", "TEAM_REPO"),
# call.GitRepoApiFactory.create(ARGS, "ROOT_ORGA", "ROOT_REPO"),
# call.GitRepo(self.team_config_git_repo_api_mock),
# call.GitRepo(self.root_config_git_repo_api_mock),
# call.GitRepo_team.get_clone_url(),
# call.logging.info("Team config repository: %s", "https://team.config.repo.git"),
# call.GitRepo_root.get_clone_url(),
# call.logging.info("Root config repository: %s", "https://root.config.repo.git"),
# call.GitRepo_team.clone(),
# call.GitRepo_team.get_full_file_path("."),
# call.os.listdir("/tmp/team-config-repo/."),
# call.os.path.join("/tmp/team-config-repo/.", "my-app"),
# call.os.path.isdir("/tmp/team-config-repo/./my-app"),
# call.logging.info("Found %s app(s) in apps repository: %s", 1, "my-app"),
# call.logging.info("Searching apps repository in root repository's 'apps/' directory..."),
# call.GitRepo_root.clone(),
# call.GitRepo_root.get_full_file_path("bootstrap/values.yaml"),
# call.yaml_file_load("/tmp/root-config-repo/bootstrap/values.yaml"),
# call.GitRepo_team.get_clone_url(),
# call.logging.info("Analyzing %s in root repository", "apps/team-non-prod.yaml"),
# call.GitRepo_root.get_full_file_path("apps/team-non-prod.yaml"),
# call.yaml_file_load("/tmp/root-config-repo/apps/team-non-prod.yaml"),
# call.logging.info("Found apps repository in %s", "apps/team-non-prod.yaml"),
# call.logging.info("Analyzing %s in root repository", "apps/other-team-non-prod.yaml"),
# call.GitRepo_root.get_full_file_path("apps/other-team-non-prod.yaml"),
# call.yaml_file_load("/tmp/root-config-repo/apps/other-team-non-prod.yaml"),
# call.logging.info("Root repository already up-to-date. I'm done here."),
# ]

SyncAppsCommand(ARGS).execute()
assert self.mock_manager.method_calls == [
call.GitRepoApiFactory.create(ARGS, "TEAM_ORGA", "TEAM_REPO"),
call.GitRepoApiFactory.create(ARGS, "ROOT_ORGA", "ROOT_REPO"),
call.GitRepo(self.team_config_git_repo_api_mock),
call.GitRepo(self.root_config_git_repo_api_mock),
call.GitRepo_team.get_clone_url(),
call.logging.info("Team config repository: %s", "https://team.config.repo.git"),
call.GitRepo_root.get_clone_url(),
call.logging.info("Root config repository: %s", "https://root.config.repo.git"),
call.GitRepo_team.clone(),
call.GitRepo_team.get_full_file_path("."),
call.os.listdir("/tmp/team-config-repo/."),
call.os.path.join("/tmp/team-config-repo/.", "my-app"),
call.os.path.isdir("/tmp/team-config-repo/./my-app"),
call.logging.info("Found %s app(s) in apps repository: %s", 1, "my-app"),
call.logging.info("Searching apps repository in root repository's 'apps/' directory..."),
call.GitRepo_root.clone(),
call.GitRepo_root.get_full_file_path("bootstrap/values.yaml"),
call.yaml_file_load("/tmp/root-config-repo/bootstrap/values.yaml"),
call.GitRepo_team.get_clone_url(),
call.logging.info("Analyzing %s in root repository", "apps/team-non-prod.yaml"),
call.GitRepo_root.get_full_file_path("apps/team-non-prod.yaml"),
call.yaml_file_load("/tmp/root-config-repo/apps/team-non-prod.yaml"),
call.logging.info("Found apps repository in %s", "apps/team-non-prod.yaml"),
call.logging.info("Analyzing %s in root repository", "apps/other-team-non-prod.yaml"),
call.GitRepo_root.get_full_file_path("apps/other-team-non-prod.yaml"),
call.yaml_file_load("/tmp/root-config-repo/apps/other-team-non-prod.yaml"),
call.logging.info("Root repository already up-to-date. I'm done here."),
]

def test_sync_apps_bootstrap_yaml_not_found(self):
self.yaml_file_load_mock.side_effect = FileNotFoundError()
Expand Down Expand Up @@ -284,3 +285,4 @@ def test_sync_apps_app_name_collission(self):
self.fail()
except GitOpsException as ex:
self.assertEqual("Application 'my-app' already exists in a different repository", str(ex))