Launchpad itself

Merge ~twom/launchpad:oci-policy-tags-hang-off-branches into launchpad:master

  1. Git
  2. lp:~twom/launchpad
  3. oci-policy-tags-hang-off-branches
  4. Merge into master
Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: 5ea55781bb5246d116e98cd14a509a7d862530c6
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~twom/launchpad:oci-policy-tags-hang-off-branches
Merge into: launchpad:master
Diff against target: 273 lines (+101/-32)
5 files modified
lib/lp/oci/interfaces/ocirecipe.py (+5/-0)
lib/lp/oci/model/ocirecipe.py (+24/-0)
lib/lp/oci/model/ociregistryclient.py (+30/-28)
lib/lp/oci/tests/test_ocirecipe.py (+22/-0)
lib/lp/oci/tests/test_ociregistryclient.py (+20/-4)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Colin Watson (community) Approve
Review via email: mp+396377@code.launchpad.net

This proposal supersedes a proposal from 2021-01-11.

Commit message

Extract tag names from branch names if in correct format

Description of the change

If the git branch format is in the expected format, add the branch/ubuntu base tag to the registry upload.
Otherwise, just add edge.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote : Posted in a previous version of this proposal

LGTM

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote : Posted in a previous version of this proposal

I'd like to see this disconnected from the distribution credentials work if possible, by using the new tag format across the board as long as the branch name is suitable (for appropriate values of "suitable"). Do you have a sense of how far away from that we are with current production data?

review: Needs Information
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :

Voting criteria not met
https://jenkins.ols.canonical.com/online-services/job/launchpad/1108/

Revision history for this message
Thiago F. Pappacena (pappacena) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
2index 131c4d3..a27c5fd 100644
3--- a/lib/lp/oci/interfaces/ocirecipe.py
4+++ b/lib/lp/oci/interfaces/ocirecipe.py
5@@ -280,6 +280,11 @@ class IOCIRecipeView(Interface):
6 "Whether everything is set up to allow uploading builds of "
7 "this OCI recipe to a registry."))
8
9+ is_valid_branch_format = Bool(
10+ title=_("Is valid branch format"), required=True, readonly=True,
11+ description=_("Whether the git branch name is the correct "
12+ "format for using as a tag name."))
13+
14 def requestBuild(requester, architecture):
15 """Request that the OCI recipe is built.
16
17diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
18index ab7f655..fd14762 100644
19--- a/lib/lp/oci/model/ocirecipe.py
20+++ b/lib/lp/oci/model/ocirecipe.py
21@@ -12,6 +12,8 @@ __all__ = [
22 'OCIRecipeSet',
23 ]
24
25+import re
26+
27 from lazr.lifecycle.event import ObjectCreatedEvent
28 import pytz
29 import six
30@@ -198,6 +200,28 @@ class OCIRecipe(Storm, WebhookTargetMixin):
31 return self._official
32
33 @property
34+ def is_valid_branch_format(self):
35+ name = self.git_ref.name
36+ split = name.split('-')
37+ # if we've not got at least two components
38+ if len(split) < 2:
39+ return False
40+ app_version = split[0:-1]
41+ ubuntu_version = split[-1]
42+ # 20.04 format
43+ ubuntu_match = re.match("\d{2}\.\d{2}", ubuntu_version)
44+ if not ubuntu_match:
45+ return False
46+ # disallow risks in app version number
47+ for risk in ["stable", "candidate", "beta", "edge"]:
48+ if risk in app_version:
49+ return False
50+ # no '/' as they're a delimiter
51+ if '/' in app_version:
52+ return False
53+ return True
54+
55+ @property
56 def build_args(self):
57 return self._build_args or {}
58
59diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
60index e528281..845abf5 100644
61--- a/lib/lp/oci/model/ociregistryclient.py
62+++ b/lib/lp/oci/model/ociregistryclient.py
63@@ -243,22 +243,22 @@ class OCIRegistryClient:
64 return data
65
66 @classmethod
67- def _calculateTag(cls, build, push_rule):
68+ def _calculateTags(cls, recipe):
69 """Work out the base tag for the image should be.
70
71- :param build: `OCIRecipeBuild` representing this build.
72 :param push_rule: `OCIPushRule` that we are using.
73 """
74- # XXX twom 2020-04-17 This needs to include OCIProjectSeries and
75- # base image name
76- return "{}".format("edge")
77+ tags = []
78+ if recipe.is_valid_branch_format:
79+ tags.append("{}_{}".format(recipe.git_ref.name, "edge"))
80+ tags.append("edge")
81+ return tags
82
83 @classmethod
84- def _getCurrentRegistryManifest(cls, http_client, push_rule):
85+ def _getCurrentRegistryManifest(cls, http_client, tag):
86 """Get the current manifest for the given push rule. If manifest
87 doesn't exist, raises HTTPError.
88 """
89- tag = cls._calculateTag(None, push_rule)
90 url = "/manifests/{}".format(tag)
91 accept = "application/vnd.docker.distribution.manifest.list.v2+json"
92 response = http_client.requestPath(
93@@ -267,7 +267,7 @@ class OCIRegistryClient:
94
95 @classmethod
96 def _uploadRegistryManifest(cls, http_client, registry_manifest,
97- push_rule, build=None):
98+ push_rule, tag, build=None):
99 """Uploads the build manifest, returning its content information.
100
101 The returned information can be used to create a Manifest list
102@@ -276,18 +276,15 @@ class OCIRegistryClient:
103
104 :return: A dict with {"digest": "sha256:xxx", "size": total_bytes}
105 """
106+ # This is
107+ # https://docs.docker.com/registry/spec/manifest-v2-2/#manifest-list
108+ # Specifically the Schema 2 manifest.
109 digest = None
110 data = json.dumps(registry_manifest)
111 size = len(data)
112 content_type = registry_manifest.get(
113 "mediaType",
114 "application/vnd.docker.distribution.manifest.v2+json")
115- if build is None:
116- # When uploading a manifest list, use the tag.
117- tag = cls._calculateTag(build, push_rule)
118- else:
119- # When uploading individual build manifests, use their digest.
120- tag = "sha256:%s" % hashlib.sha256(data).hexdigest()
121 try:
122 manifest_response = http_client.requestPath(
123 "/manifests/{}".format(tag),
124@@ -311,7 +308,7 @@ class OCIRegistryClient:
125
126 @classmethod
127 def _upload_to_push_rule(
128- cls, push_rule, build, manifest, digests, preloaded_data):
129+ cls, push_rule, build, manifest, digests, preloaded_data, tag):
130 http_client = RegistryHTTPClient.getInstance(push_rule)
131
132 for section in manifest:
133@@ -346,7 +343,7 @@ class OCIRegistryClient:
134
135 # Upload the registry manifest
136 manifest = cls._uploadRegistryManifest(
137- http_client, registry_manifest, push_rule, build)
138+ http_client, registry_manifest, push_rule, tag, build)
139
140 # Save the uploaded manifest location, so we can use it in case
141 # this is a multi-arch image upload.
142@@ -376,11 +373,13 @@ class OCIRegistryClient:
143
144 exceptions = []
145 for push_rule in build.recipe.push_rules:
146- try:
147- cls._upload_to_push_rule(
148- push_rule, build, manifest, digests, preloaded_data)
149- except Exception as e:
150- exceptions.append(e)
151+ for tag in cls._calculateTags(build.recipe):
152+ try:
153+ cls._upload_to_push_rule(
154+ push_rule, build, manifest, digests, preloaded_data,
155+ tag)
156+ except Exception as e:
157+ exceptions.append(e)
158 if len(exceptions) == 1:
159 raise exceptions[0]
160 elif len(exceptions) > 1:
161@@ -388,7 +387,7 @@ class OCIRegistryClient:
162
163 @classmethod
164 def makeMultiArchManifest(cls, http_client, push_rule, build_request,
165- uploaded_builds):
166+ uploaded_builds, tag):
167 """Returns the multi-arch manifest content including all uploaded
168 builds of the given build_request.
169 """
170@@ -402,7 +401,7 @@ class OCIRegistryClient:
171
172 try:
173 current_manifest = cls._getCurrentRegistryManifest(
174- http_client, push_rule)
175+ http_client, tag)
176 # Check if the current manifest is not an incompatible version.
177 version = current_manifest.get("schemaVersion", 1)
178 if version < 2 or "manifests" not in current_manifest:
179@@ -481,11 +480,14 @@ class OCIRegistryClient:
180 if not uploaded_builds:
181 return
182 for push_rule in build_request.recipe.push_rules:
183- http_client = RegistryHTTPClient.getInstance(push_rule)
184- multi_manifest_content = cls.makeMultiArchManifest(
185- http_client, push_rule, build_request, uploaded_builds)
186- cls._uploadRegistryManifest(
187- http_client, multi_manifest_content, push_rule, build=None)
188+ for tag in cls._calculateTags(build_request.recipe):
189+ http_client = RegistryHTTPClient.getInstance(push_rule)
190+ multi_manifest_content = cls.makeMultiArchManifest(
191+ http_client, push_rule, build_request, uploaded_builds,
192+ tag)
193+ cls._uploadRegistryManifest(
194+ http_client, multi_manifest_content, push_rule, tag,
195+ build=None)
196
197
198 class OCIRegistryAuthenticationError(Exception):
199diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
200index a0b1c59..59bd465 100644
201--- a/lib/lp/oci/tests/test_ocirecipe.py
202+++ b/lib/lp/oci/tests/test_ocirecipe.py
203@@ -870,6 +870,28 @@ class TestOCIRecipeProcessors(TestCaseWithFactory):
204 self.assertContentEqual(
205 [self.default_procs[0], self.arm, hppa], recipe.processors)
206
207+ def test_valid_branch_format(self):
208+ [git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v1.0-20.04"])
209+ recipe = self.factory.makeOCIRecipe(git_ref=git_ref)
210+ self.assertTrue(recipe.is_valid_branch_format)
211+
212+ def test_valid_branch_format_invalid(self):
213+ [git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v1.0-foo"])
214+ recipe = self.factory.makeOCIRecipe(git_ref=git_ref)
215+ self.assertFalse(recipe.is_valid_branch_format)
216+
217+ def test_valid_branch_format_invalid_uses_risk(self):
218+ for risk in ["stable", "candidate", "beta", "edge"]:
219+ path = "refs/heads/{}-20.04".format(risk)
220+ [git_ref] = self.factory.makeGitRefs(paths=[path])
221+ recipe = self.factory.makeOCIRecipe(git_ref=git_ref)
222+ self.assertFalse(recipe.is_valid_branch_format)
223+
224+ def test_valid_branch_format_invalid_with_slash(self):
225+ [git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v1.0/bar-foo"])
226+ recipe = self.factory.makeOCIRecipe(git_ref=git_ref)
227+ self.assertFalse(recipe.is_valid_branch_format)
228+
229
230 class TestOCIRecipeSet(TestCaseWithFactory):
231
232diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
233index ecfaee3..bda327f 100644
234--- a/lib/lp/oci/tests/test_ociregistryclient.py
235+++ b/lib/lp/oci/tests/test_ociregistryclient.py
236@@ -181,6 +181,14 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
237 )
238 responses.add("PUT", manifests_url, status=status_code, json=json)
239
240+ # recipes that the git branch name matches the correct format
241+ manifests_url = "{}/v2/{}/manifests/{}_edge".format(
242+ push_rule.registry_credentials.url,
243+ push_rule.image_name,
244+ push_rule.recipe.git_ref.name
245+ )
246+ responses.add("PUT", manifests_url, status=status_code, json=json)
247+
248 @responses.activate
249 def test_upload(self):
250 self._makeFiles()
251@@ -338,10 +346,18 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
252 'diff_id_1': Equals(self.layer_files[0].library_file),
253 'diff_id_2': Equals(self.layer_files[1].library_file)})}))
254
255- def test_calculateTag(self):
256- result = self.client._calculateTag(
257- self.build, self.build.recipe.push_rules[0])
258- self.assertEqual("edge", result)
259+ def test_calculateTags_invalid_format(self):
260+ [git_ref] = self.factory.makeGitRefs(paths=["refs/heads/invalid"])
261+ self.build.recipe.git_ref = git_ref
262+ result = self.client._calculateTags(self.build.recipe)
263+ self.assertThat(result, MatchesListwise([Equals("edge")]))
264+
265+ def test_calculateTags_valid_format(self):
266+ [git_ref] = self.factory.makeGitRefs(paths=["refs/heads/v1.0-20.04"])
267+ self.build.recipe.git_ref = git_ref
268+ result = self.client._calculateTags(self.build.recipe)
269+ self.assertThat(result, MatchesListwise(
270+ [Equals("v1.0-20.04_edge"), Equals("edge")]))
271
272 def test_build_registry_manifest(self):
273 self._makeFiles()

Subscribers

People subscribed via source and target branches

to status/vote changes: