PR#79: New topic scheme, now including namespace. - fedora-ci/messages

fedora-ci / messages

#79 New topic scheme, now including namespace.

Merged 4 years ago by ralph. Opened 4 years ago by ralph.

namespace-topic-v2 into master

Add text clarifying why the namespace change is important.

Ralph Bean • 4 years ago

47fd469

Clean up old topic references.

Ralph Bean • 4 years ago

ae559f5

Mention that the namespace in the body must match the topic.

Ralph Bean • 4 years ago

a614b23

Describe new topic scheme in the README.

Ralph Bean • 4 years ago

bd3255c

README.md

file modified

+16 -3

		`@@ -54,7 +54,7 @@`
		`Virtual topic namespace`
		`-----------------------`

		`- The messages will use unique UMB virtual topic namespaces to mitigate`
		`+ The messages will use unique UMB virtual topic hierarchies to mitigate`
		`collisions with other systems:`

		`UMB: /topic/VirtualTopic.eng.ci`
		`@@ -66,8 +66,15 @@`
		`For all the tests on all the artifacts the systems will use this`
		`topic naming convention:`

		`- UMB: /topic/VirtualTopic.eng.ci.<artifact>.<event>.{queued,running,complete,error}`
		`- fedmsg: org.fedoraproject.prod.ci.<artifact>.<event>.{queued,running,complete,error}`
		`+ UMB: /topic/VirtualTopic.eng.ci.<namespace>.<artifact>.<event>.{queued,running,complete,error}`
		`+ fedmsg: org.fedoraproject.prod.ci.<namespace>.<artifact>.<event>.{queued,running,complete,error}`
		`+`
		+ where `namespace` is any string that denotes the source of the message, required to be able to use
		`+ AMQP ACLs to secure publication rights between various publishing teams, for example:`
		`+`
		`+ * osci - a message from the "Operating System CI" team's systems.`
		`+ * cvp - a message from the "Container Verification Pipeline".`
		`+ * art - a message from the jenkins master belonging to the "Automatic Release Team".`

		where `artifact` is one of the defined build artifacts, for
		`example:`
		`@@ -84,6 +91,12 @@`
		`* test - for testing progression and results`
		`* gate - for gating events of artifacts, i.e. events blocking progression of the artifact in the release pipeline`

		+ Note that an older version of this schema omitted the `namespace` field. Messages bearing those
		`+ topics will still appear for some time while teams migrate to the modern topic structure above.`
		`+`
		`+ UMB: /topic/VirtualTopic.eng.ci.<artifact>.<event>.{queued,running,complete,error}`
		`+ fedmsg: org.fedoraproject.prod.ci.<artifact>.<event>.{queued,running,complete,error}`
		`+`
		`Message Anatomy`
		`---------------`

schemas/brew-build.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the brew-build.test.complete topic.`
		`+ Test failures should be exposed under the <namespace>.brew-build.test.complete topic.`
		`This is a mandatory message.`

		`properties:`

schemas/component-version.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the component-version.test.error topic.`
		`+ Test failures should be exposed under the <namespace>.component-version.test.error topic.`
		`This is a mandatory message.`

		`properties:`

schemas/container-image.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the brew-build.test.error topic.`
		`+ Test failures should be exposed under the <namespace>.brew-build.test.error topic.`
		`This is a mandatory message.`

		`properties:`

schemas/koji-build.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the koji-build.test.error topic.`
		`+ Test failures should be exposed under the <namespace>.koji-build.test.error topic.`
		`This is a mandatory message.`

		`properties:`

schemas/product-build.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the product-build.test.complete`
		`+ Test failures should be exposed under the <namespace>.product-build.test.complete`
		`topic. This is a mandatory message.`

		`properties:`

schemas/productmd-compose.test.error.yaml

file modified

+1 -1

		`@@ -4,7 +4,7 @@`
		`description:`
		`Testing has aborted, it was not finished, e.g. because of infrastructure`
		`error, CI system error, etc. Note that a test failure is not an error.`
		`- Test failures should be exposed under the productmd-compose.test.complete`
		`+ Test failures should be exposed under the <namespace>.productmd-compose.test.complete`
		`topic. This is a mandatory message.`

		`properties:`

schemas/pull-request.test.error.yaml

file modified

+1 -1

		`@@ -5,7 +5,7 @@`
		`Testing of the pull request has aborted, it was not finished, e.g.`
		`because of an infrastructure error or CI system error. Note that a`
		`test failure is not an error. Test failures should be exposed under`
		`- the pull-request.test.error topic. This is a mandatory message.`
		`+ the <namespace>.pull-request.test.error topic. This is a mandatory message.`

		`properties:`
		`contact:`

schemas/test-common.yaml

file modified

		`@@ -52,6 +52,12 @@`
		`description:`
		`A namespace used to prefix the ResultsDB testcase name.`
		`This is usually something identifying your team.`
		`+`
		`+ It must match the namespace used in the topic hierarchy. Otherwise,`
		`+ a system with rights to publish to one topic namespace may publish results`
		`+ for testcase names belonging to another namespace. The impact of this depends`
		`+ on how test results are used, typically for gating or automatic promotion of`
		`+ content.`
		`examples:`
		`- osci`
		`- rtt`

ralph commented 4 years ago

The point of this change is to allow sensible ACLs controlling who can publish
what kind of message.

Today, a publishing client is granted access to publish to a topic hierarchy
(say, ci.>) which allows one publisher to write test results that we would
expect to come from a different publisher. The new namespace in the topic
hierarchy allows us to reduce the rights given to publisher identities such
that they can only publish test results for test cases that we would expect
(i.e., OSCI can only publish osci.* test cases.)

adamwill commented 4 years ago

This seems to go against the whole original spirit of this scheme, though, doesn't it? The whole idea is that it doesn't matter what test system publishes a message. If we start putting test system names in the topics, it breaks that idea. The idea was that you can just listen out for ci.koji-build.test-complete messages, or whatever, and be sure you're getting all the messages for (compliant) tests of Koji builds. This gets completely broken if now you have to listen out for ci.osci.koji-build.test-complete, ci.cvp.koji-build.test.complete and ci.art.koji-build.test.complete - and eternally be on the lookout for some new namespace appearing, whose messages you won't get until you add yet another subscription...

Edited 4 years ago by adamwill

ralph commented 4 years ago

Ah - that ability is maintained with AMQP which allows you to subscribe to wildcard topics - you can listen to ci.*.koji-build.test-complete. Old zeromq-based fedmsg doesn't allow that, though.

mvadkert commented 4 years ago

Yeah, I believe both AMQP versions we use (1.x internally and 0.9.x in Fedora) should support that

mvadkert commented on line 23 of README.md 4 years ago

Can we document that the namespace is required to be able to use AMQP ACLs to secure the topics between various namespaces?
Maybe that is granted

mvadkert commented on line 6 of schemas/test-common.yaml 4 years ago

Same here, let's add more context WHY this is important

mvadkert commented 4 years ago

@ralph 2 small enhancements needed and it is :thumbsup: from me

Let's also wait for @adamwill to confirm that with AMQP this is a solvable problem ...

1 new commit added

Add text clarifying why the namespace change is important.

4 years ago

adamwill commented 4 years ago

Oh, yeah, I guess wildcard subscriptions help, though it also means you have to be more careful with checks on the topic inside the consumers. I still regret the loss of elegance compared to the original concept here, but I guess it's acceptable.

Hmm, one thing: can you subscribe to e.g. ci.*koji-build.test-complete so you will get ci.(whatever).koji-build.test-complete messages and ci.koji-build.test-complete messages?