Skip to content

Commit 865e094

Browse files
authored
Merge branch 'main' into rest
2 parents 95460f3 + 6b1e458 commit 865e094

File tree

9 files changed

+434
-11
lines changed

9 files changed

+434
-11
lines changed

.github/workflows/add-remove-labels.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ jobs:
1919
contains(github.event.comment.body, '/verified') ||
2020
contains(github.event.comment.body, '/lgtm') ||
2121
contains(github.event.comment.body, '/hold') ||
22-
contains(github.event.comment.body, '/cherry-pick')
22+
contains(github.event.comment.body, '/cherry-pick') ||
23+
contains(github.event.comment.body, '/build-push-pr-image')
2324
runs-on: ubuntu-latest
2425

2526
steps:
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
name: Delete PR Image On PR Close Action
2+
3+
on:
4+
pull_request_target:
5+
types: [closed]
6+
7+
permissions:
8+
pull-requests: write
9+
contents: write
10+
issues: write
11+
12+
jobs:
13+
delete-quay-tag:
14+
runs-on: ubuntu-latest
15+
steps:
16+
- name: Install regctl
17+
run: |
18+
curl -LO https://github.com/regclient/regclient/releases/latest/download/regctl-linux-amd64
19+
chmod +x regctl-linux-amd64
20+
sudo mv regctl-linux-amd64 /usr/local/bin/regctl
21+
regctl version
22+
23+
- name: Configure regctl authentication
24+
run: |
25+
regctl registry login quay.io -u ${{ secrets.QUAY_USERNAME }} -p ${{ secrets.QUAY_PASSWORD }}
26+
echo "PR number: ${{ github.event.pull_request.number }}"
27+
echo "TAG_TO_DELETE=$(regctl tag ls quay.io/opendatahub/opendatahub-tests --include pr-${{ github.event.pull_request.number }})" >> $GITHUB_ENV
28+
- name: Delete Quay Tag
29+
if: env.TAG_TO_DELETE != ''
30+
run: |
31+
echo "Deleting tag '$TAG_TO_DELETE' from repository..."
32+
regctl tag rm quay.io/opendatahub/opendatahub-tests:pr-${{ github.event.pull_request.number }}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
name: Push Container Image On PR Comment
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
permissions:
8+
pull-requests: write
9+
contents: write
10+
issues: write
11+
12+
jobs:
13+
push-container-on-comment:
14+
if: contains(github.event.comment.body, '/build-push-pr-image')
15+
runs-on: ubuntu-latest
16+
steps:
17+
- name: Checkout Repository
18+
uses: actions/checkout@v4
19+
- name: Install uv
20+
uses: astral-sh/setup-uv@v6
21+
22+
- name: Check if the user is authorized
23+
env:
24+
GITHUB_TOKEN: ${{ secrets.RHODS_CI_BOT_PAT }}
25+
GITHUB_PR_NUMBER: ${{ github.event.issue.number }}
26+
GITHUB_EVENT_ACTION: ${{ github.event.action }}
27+
GITHUB_EVENT_REVIEW_STATE: ${{ github.event.review.state }}
28+
GITHUB_EVENT_NAME: ${{ github.event_name }}
29+
COMMENT_BODY: ${{ github.event.comment.body }}
30+
REVIEW_COMMENT_BODY: ${{ github.event.review.body }}
31+
GITHUB_USER_LOGIN: ${{ github.event.sender.login }}
32+
ACTION: "push-container-on-comment"
33+
run: uv run python .github/workflows/scripts/pr_workflow.py
34+
- name: Set env TAG for image
35+
run: |
36+
echo "TAG=pr-${{ github.event.issue.number }}" >> "$GITHUB_ENV"
37+
- name: Build Image to push
38+
id: build-image
39+
uses: redhat-actions/buildah-build@v2
40+
with:
41+
image: opendatahub-tests
42+
tags: ${{ env.TAG }}
43+
containerfiles: |
44+
./Dockerfile
45+
- name: Push To Image Registry
46+
id: push-to-registry
47+
uses: redhat-actions/push-to-registry@v2
48+
with:
49+
image: ${{ steps.build-image.outputs.image }}
50+
tags: ${{ steps.build-image.outputs.tags }}
51+
registry: quay.io/opendatahub
52+
username: ${{ secrets.QUAY_USERNAME }}
53+
password: ${{ secrets.QUAY_PASSWORD }}
54+
55+
- name: Add comment to PR
56+
if: always()
57+
env:
58+
URL: ${{ github.event.issue.comments_url }}
59+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
60+
run: |
61+
curl \
62+
-X POST \
63+
$URL \
64+
-H "Content-Type: application/json" \
65+
-H "Authorization: token $GITHUB_TOKEN" \
66+
--data '{ "body": "Status of building tag ${{ env.TAG }}: ${{ steps.build-image.outcome }}. \nStatus of pushing tag ${{ env.TAG }} to image registry: ${{ steps.push-to-registry.outcome }}." }'

.github/workflows/scripts/pr_workflow.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ class SupportedActions:
3434
add_remove_labels_action_name: str = "add-remove-labels"
3535
pr_size_action_name: str = "add-pr-size-label"
3636
welcome_comment_action_name: str = "add-welcome-comment-set-assignee"
37+
build_push_pr_image_action_name: str = "push-container-on-comment"
3738
supported_actions: set[str] = {
3839
pr_size_action_name,
3940
add_remove_labels_action_name,
4041
welcome_comment_action_name,
42+
build_push_pr_image_action_name,
4143
}
4244

4345
def __init__(self) -> None:
@@ -58,7 +60,7 @@ def __init__(self) -> None:
5860
def verify_base_config(self) -> None:
5961
if not self.action or self.action not in self.SupportedActions.supported_actions:
6062
sys.exit(
61-
"`ACTION` is not set in workflow or is not supported. "
63+
f"{self.action} is not set in workflow or is not supported. "
6264
f"Supported actions: {self.SupportedActions.supported_actions}"
6365
)
6466

@@ -97,10 +99,9 @@ def __init__(self) -> None:
9799
self.comment_body = os.getenv("REVIEW_COMMENT_BODY", "")
98100
self.last_commit = list(self.pr.get_commits())[-1]
99101
self.last_commit_sha = self.last_commit.sha
100-
101102
self.verify_labeler_config()
102103

103-
def verify_allowed_user(self) -> None:
104+
def verify_allowed_user(self) -> bool:
104105
org: Organization = self.gh_client.get_organization("opendatahub-io")
105106
# slug is the team name with replaced special characters,
106107
# all words to lowercase and spaces replace with a -
@@ -109,9 +110,10 @@ def verify_allowed_user(self) -> None:
109110
# check if the user is a member of opendatahub-tests-contributors
110111
membership = team.get_team_membership(member=self.user_login)
111112
LOGGER.info(f"User {self.user_login} is a member of the test contributor team. {membership}")
113+
return True
112114
except UnknownObjectException:
113115
LOGGER.error(f"User {self.user_login} is not allowed for this action. Exiting.")
114-
sys.exit(0)
116+
return False
115117

116118
def verify_labeler_config(self) -> None:
117119
if self.action == self.SupportedActions.add_remove_labels_action_name and self.event_name in (
@@ -131,9 +133,13 @@ def run_pr_label_action(self) -> None:
131133
if self.action == self.SupportedActions.pr_size_action_name:
132134
self.set_pr_size()
133135

136+
if self.action == self.SupportedActions.build_push_pr_image_action_name:
137+
if not self.verify_allowed_user():
138+
sys.exit(1)
139+
134140
if self.action == self.SupportedActions.add_remove_labels_action_name:
135-
self.verify_allowed_user()
136-
self.add_remove_pr_labels()
141+
if self.verify_allowed_user():
142+
self.add_remove_pr_labels()
137143

138144
if self.action == self.SupportedActions.welcome_comment_action_name:
139145
self.add_welcome_comment_set_assignee()

docs/GITHUB_WORKFLOWS.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,12 @@
1010

1111
### On user action
1212
- Add to or remove a label from PR; supported labels: `wip`, `lgtm`, `verified`, and `hold`.
13-
To add a new label, add `/<label name>` in a comment.
14-
To remove a label, add `/<label name> cancel` in a comment.
13+
- To add a new label, add `/<label name>` in a comment.
14+
- To remove a label, add `/<label name> cancel` in a comment.
1515
`verified` and `lgtm` are removed on new commits.
16+
- To build and push image to quay, add `/build-push-pr-image` in a comment.
17+
This would create an image with tag pr-<pr_number> to quay repository. This image tag,
18+
however would be deleted on PR merge or close action.
1619

1720
## How to add a new workflow
1821
1. Create a new file in `.github/workflows` directory.
@@ -25,7 +28,6 @@
2528
## To be added
2629
- Block merging if not all defined checks pass. For example: a `verified` label was added and at least 2 approvals.
2730
- When a PR is opened, add reviewers (requires updates to OWNERS file(s))
28-
- When a PR is reviewed/commented by a user who's not the PR owner, add `reviewed|commented|approved-by-<username>` label
2931
- When a PR is ready to be merged (all checks passed), add `ready-to-merge` label
3032
- If a label is missing from the repository (i.e was manually deleted), add it back (label colors should be defined as well)
3133
- Tests

tests/model_registry/conftest.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from ocp_resources.deployment import Deployment
1313

1414
from ocp_resources.model_registry import ModelRegistry
15-
import schemathesis.schemas
1615
from schemathesis.specs.openapi.schemas import BaseOpenAPISchema
1716
from schemathesis.generation.stateful.state_machine import APIStateMachine
1817
from schemathesis.core.transport import Response
Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
import pytest
2+
import shlex
3+
import subprocess
4+
import os
5+
from typing import Generator, List, Dict, Any
6+
from ocp_resources.namespace import Namespace
7+
from ocp_resources.service_account import ServiceAccount
8+
from ocp_resources.role_binding import RoleBinding
9+
from ocp_resources.role import Role
10+
from kubernetes.dynamic import DynamicClient
11+
from pyhelper_utils.shell import run_command
12+
from tests.model_registry.utils import generate_random_name, generate_namespace_name
13+
from simple_logger.logger import get_logger
14+
from tests.model_registry.constants import MR_INSTANCE_NAME
15+
16+
17+
LOGGER = get_logger(name=__name__)
18+
DEFAULT_TOKEN_DURATION = "10m"
19+
20+
21+
@pytest.fixture(scope="function")
22+
def sa_namespace(request: pytest.FixtureRequest, admin_client: DynamicClient) -> Generator[Namespace, None, None]:
23+
"""
24+
Creates a temporary namespace using a context manager for automatic cleanup.
25+
Function scope ensures a fresh namespace for each test needing it.
26+
"""
27+
test_file = os.path.relpath(request.fspath.strpath, start=os.path.dirname(__file__))
28+
ns_name = generate_namespace_name(file_path=test_file)
29+
LOGGER.info(f"Creating temporary namespace: {ns_name}")
30+
with Namespace(client=admin_client, name=ns_name) as ns:
31+
ns.wait_for_status(status=Namespace.Status.ACTIVE, timeout=120)
32+
yield ns
33+
34+
35+
@pytest.fixture(scope="function")
36+
def service_account(admin_client: DynamicClient, sa_namespace: Namespace) -> Generator[ServiceAccount, None, None]:
37+
"""
38+
Creates a ServiceAccount within the temporary namespace using a context manager.
39+
Function scope ensures it's tied to the lifetime of sa_namespace for that test.
40+
"""
41+
sa_name = generate_random_name(prefix="mr-test-user")
42+
LOGGER.info(f"Creating ServiceAccount: {sa_name} in namespace {sa_namespace.name}")
43+
with ServiceAccount(client=admin_client, name=sa_name, namespace=sa_namespace.name, wait_for_resource=True) as sa:
44+
yield sa
45+
46+
47+
@pytest.fixture(scope="function")
48+
def sa_token(service_account: ServiceAccount) -> str:
49+
"""
50+
Retrieves a short-lived token for the ServiceAccount using 'oc create token'.
51+
Function scope because token is temporary and tied to the SA for that test.
52+
"""
53+
sa_name = service_account.name
54+
namespace = service_account.namespace
55+
LOGGER.info(f"Retrieving token for ServiceAccount: {sa_name} in namespace {namespace}")
56+
try:
57+
cmd = f"oc create token {sa_name} -n {namespace} --duration={DEFAULT_TOKEN_DURATION}"
58+
LOGGER.debug(f"Executing command: {cmd}")
59+
res, out, err = run_command(command=shlex.split(cmd), verify_stderr=False, check=True, timeout=30)
60+
token = out.strip()
61+
if not token:
62+
raise ValueError("Retrieved token is empty after successful command execution.")
63+
64+
LOGGER.info(f"Successfully retrieved token for SA '{sa_name}'")
65+
return token
66+
67+
except Exception as e: # Catch all exceptions from the try block
68+
error_type = type(e).__name__
69+
log_message = (
70+
f"Failed during token retrieval for SA '{sa_name}' in namespace '{namespace}'. "
71+
f"Error Type: {error_type}, Message: {str(e)}"
72+
)
73+
if isinstance(e, subprocess.CalledProcessError):
74+
# Add specific details for CalledProcessError
75+
# run_command already logs the error if log_errors=True and returncode !=0,
76+
# but we can add context here.
77+
stderr_from_exception = e.stderr.strip() if e.stderr else "N/A"
78+
log_message += f". Exit Code: {e.returncode}. Stderr from exception: {stderr_from_exception}"
79+
elif isinstance(e, subprocess.TimeoutExpired):
80+
timeout_value = getattr(e, "timeout", "N/A")
81+
log_message += f". Command timed out after {timeout_value} seconds."
82+
elif isinstance(e, FileNotFoundError):
83+
# This occurs if 'oc' is not found.
84+
# e.filename usually holds the name of the file that was not found.
85+
command_not_found = e.filename if hasattr(e, "filename") and e.filename else shlex.split(cmd)[0]
86+
log_message += f". Command '{command_not_found}' not found. Is it installed and in PATH?"
87+
88+
LOGGER.error(log_message, exc_info=True) # exc_info=True adds stack trace to the log
89+
raise
90+
91+
92+
# --- RBAC Fixtures ---
93+
94+
95+
@pytest.fixture(scope="function")
96+
def mr_access_role(
97+
admin_client: DynamicClient,
98+
model_registry_namespace: str,
99+
sa_namespace: Namespace,
100+
) -> Generator[Role, None, None]:
101+
"""
102+
Creates the MR Access Role using direct constructor parameters and a context manager.
103+
"""
104+
role_name = f"registry-user-{MR_INSTANCE_NAME}-{sa_namespace.name[:8]}"
105+
LOGGER.info(f"Defining Role: {role_name} in namespace {model_registry_namespace}")
106+
107+
role_rules: List[Dict[str, Any]] = [
108+
{
109+
"apiGroups": [""],
110+
"resources": ["services"],
111+
"resourceNames": [MR_INSTANCE_NAME], # Grant access only to the specific MR service object
112+
"verbs": ["get"],
113+
}
114+
]
115+
role_labels = {
116+
"app.kubernetes.io/component": "model-registry-test-rbac",
117+
"test.opendatahub.io/namespace": sa_namespace.name,
118+
}
119+
120+
LOGGER.info(f"Attempting to create Role: {role_name} with rules and labels.")
121+
with Role(
122+
client=admin_client,
123+
name=role_name,
124+
namespace=model_registry_namespace,
125+
rules=role_rules,
126+
label=role_labels,
127+
wait_for_resource=True,
128+
) as role:
129+
LOGGER.info(f"Role {role.name} created successfully.")
130+
yield role
131+
LOGGER.info(f"Role {role.name} deletion initiated by context manager.")
132+
133+
134+
@pytest.fixture(scope="function")
135+
def mr_access_role_binding(
136+
admin_client: DynamicClient,
137+
model_registry_namespace: str,
138+
mr_access_role: Role,
139+
sa_namespace: Namespace,
140+
) -> Generator[RoleBinding, None, None]:
141+
"""
142+
Creates the MR Access RoleBinding using direct constructor parameters and a context manager.
143+
"""
144+
binding_name = f"{mr_access_role.name}-binding"
145+
146+
LOGGER.info(
147+
f"Defining RoleBinding: {binding_name} linking Group 'system:serviceaccounts:{sa_namespace.name}' "
148+
f"to Role '{mr_access_role.name}' in namespace {model_registry_namespace}"
149+
)
150+
binding_labels = {
151+
"app.kubernetes.io/component": "model-registry-test-rbac",
152+
"test.opendatahub.io/namespace": sa_namespace.name,
153+
}
154+
155+
LOGGER.info(f"Attempting to create RoleBinding: {binding_name} with labels.")
156+
with RoleBinding(
157+
client=admin_client,
158+
name=binding_name,
159+
namespace=model_registry_namespace,
160+
# Subject parameters
161+
subjects_kind="Group",
162+
subjects_name=f"system:serviceaccounts:{sa_namespace.name}",
163+
subjects_api_group="rbac.authorization.k8s.io", # This is the default apiGroup for Group kind
164+
# Role reference parameters
165+
role_ref_kind="Role",
166+
role_ref_name=mr_access_role.name,
167+
label=binding_labels,
168+
wait_for_resource=True,
169+
) as binding:
170+
LOGGER.info(f"RoleBinding {binding.name} created successfully.")
171+
yield binding
172+
LOGGER.info(f"RoleBinding {binding.name} deletion initiated by context manager.")

0 commit comments

Comments
 (0)