Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

build: allow to define additional configs #728

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 32 additions & 0 deletions asu/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,23 @@ def build(build_request: BuildRequest, job=None):
},
)

if build_request.configs:
log.debug("Found extra configs")
configs = ""
for config in build_request.configs:
configs += f"{config}\n"

(bin_dir / ".config_local").write_text(configs)

mounts.append(
{
"type": "bind",
"source": str(bin_dir / ".config_local"),
"target": "/builder/.config_local",
"read_only": True,
}
)

log.debug("Mounts: %s", mounts)

container = podman.containers.create(
Expand Down Expand Up @@ -243,6 +260,21 @@ def build(build_request: BuildRequest, job=None):
packages_hash: str = get_packages_hash(manifest.keys())
log.debug(f"Packages Hash: {packages_hash}")

if build_request.configs:
log.info("Appling local configs")
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "Applying"

returncode, job.meta["stdout"], job.meta["stderr"] = run_cmd(
container,
[
"sh",
"-c",
(
"for config in $(grep '#' .config_local | awk '{print $2}'); do sed -i 's/'$config'.*//' .config ; done; cat .config_local >> .config"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do this filtering when you are creating the .config_local file?

),
],
)
if returncode:
report_error(job, "Could not apply local configs")

job.meta["build_cmd"] = [
"make",
"image",
Expand Down
21 changes: 21 additions & 0 deletions asu/build_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
STRING_PATTERN = r"^[\w.,-]*$"
TARGET_PATTERN = r"^[\w]*/[\w]*$"
PKG_VERSION_PATTERN = r"^[\w.,~-]*$"
CONFIG_PATTERN = (
r"^(# CONFIG_[\w_.-]+ is not set|CONFIG_[\w_.-]+=([\w_.-]+$|\"[^\"][\w_.-]+\"))$"
)


class BuildRequest(BaseModel):
Expand Down Expand Up @@ -158,3 +161,21 @@ class BuildRequest(BaseModel):
""".strip(),
),
] = None
configs: Annotated[
list[Annotated[str, Field(pattern=CONFIG_PATTERN)]],
Field(
examples=[
[
"CONFIG_VERSION_DIST=MyRouterOS",
"CONFIG_VERSION_NUMBER=1.6",
"CONFIG_TARGET_ROOTFS_TARGZ=y",
"CONFIG_TARGET_ROOTFS_JFFS2=y",
"# CONFIG_TARGET_ROOTFS_SQUASHFS is not set",
]
],
description="""
List of configs, only the few ones not related to kernel/packages
as they will not be recompiled.
""".strip(),
),
] = []
8 changes: 8 additions & 0 deletions asu/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ class Settings(BaseSettings):
server_stats: str = "/stats"
log_level: str = "INFO"
squid_cache: bool = False
configs_allowed: list = []
# configs_allowed: list = [
# 'CONFIG_VERSION_DIST',
# 'CONFIG_VERSION_NUMBER',
# 'CONFIG_TARGET_ROOTFS_TARGZ',
# 'CONFIG_TARGET_ROOTFS_JFFS2',
# 'CONFIG_TARGET_ROOTFS_SQUASHFS'
# ]


settings = Settings()
10 changes: 10 additions & 0 deletions asu/routers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
get_request_hash,
)

import re

configs_extract = r"^# (CONFIG_[\w_.-]) is not set|(CONFIG_[\w_.-]+)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the second clause also be anchored? As is, the second clause matches anywhere in the line.


router = APIRouter()


Expand Down Expand Up @@ -71,6 +75,12 @@ def validate_request(build_request: BuildRequest) -> tuple[dict, int]:
if build_request.distro not in get_distros():
return validation_failure(f"Unsupported distro: {build_request.distro}")

if build_request.configs:
for config in build_request.configs:
config_key = re.search(configs_extract, config)
if config_key[0] not in settings.configs_allowed:
return validation_failure("Illegal config requested: {config_key[0]}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be an f-string to evaluate the expression.


branch = get_branch(build_request.version)["name"]

r = get_redis_client()
Expand Down
13 changes: 13 additions & 0 deletions asu/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@ def get_file_hash(path: str) -> str:
return h.hexdigest()


def get_configs_hash(configs: list) -> str:
"""Return sha256sum of configs list
Duplicate configs are automatically removed and the list is sorted to be
reproducible
Args:
configs (list): list of configs
Returns:
str: hash of `req`
"""
return get_str_hash(" ".join(sorted(list(set(configs)))))
Copy link
Contributor

Choose a reason for hiding this comment

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

You could leave out that extra list call, just say sorted(set( as that has the same effect and eliminates an extra list-ification.



def get_manifest_hash(manifest: dict[str, str]) -> str:
"""Return sha256sum of package manifest

Expand Down Expand Up @@ -132,6 +144,7 @@ def get_request_hash(build_request: BuildRequest) -> str:
build_request.target,
build_request.profile.replace(",", "_"),
get_packages_hash(build_request.packages),
get_configs_hash(build_request.configs),
get_manifest_hash(build_request.packages_versions),
str(build_request.diff_packages),
"", # build_request.filesystem
Expand Down