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

Handle webhooks from Harbor container repo #44

Closed
rossigee opened this issue Apr 14, 2024 · 11 comments · Fixed by #43 · May be fixed by #46
Closed

Handle webhooks from Harbor container repo #44

rossigee opened this issue Apr 14, 2024 · 11 comments · Fixed by #43 · May be fixed by #46
Labels
enhancement New feature or request

Comments

@rossigee
Copy link
Contributor

Harbor's webhook format is different to the ones currently supported.

{
  "type": "PUSH_ARTIFACT",
  "occur_at": 1680501893,
  "operator": "harbor-jobservice",
  "event_data": {
    "resources": [
      {
        "digest": "sha256:954b378c375d852eb3c63ab88978f640b4348b01c1b3456a024a81536dafbbf4",
        "tag": "sha256:954b378c375d852eb3c63ab88978f640b4348b01c1b3456a024a81536dafbbf4",
        "resource_url": "localhost/harbor/alpine@sha256:954b378c375d852eb3c63ab88978f640b4348b01c1b3456a024a81536dafbbf4"
      }
    ],
    "repository": {
      "date_created": 1680501893,
      "name": "alpine",
      "namespace": "harbor",
      "repo_full_name": "harbor/alpine",
      "repo_type": "private"
    }
  }
}
@github-actions github-actions bot added the triage requires review label Apr 14, 2024
@bluebrown bluebrown linked a pull request Apr 14, 2024 that will close this issue
@bluebrown
Copy link
Owner

bluebrown commented Apr 14, 2024

Hi @rossigee, I was very quick with merging, and I was not very focused. I just took a look at the harbor payload, and the way you show it, it probably won't work.

If the resource URL is actually:

localhost/harbor/alpine@sha256:954b378c375d852eb3c63ab88978f640b4348b01c1b3456a024a81536dafbbf4

Then its missing the tag, so it would never match any options.

@bluebrown
Copy link
Owner

bluebrown commented Apr 14, 2024

Well, it seems that harbor is not including a tag in the payload. goharbor/harbor#19982

@bluebrown bluebrown added enhancement New feature or request and removed triage requires review labels Apr 14, 2024
@rossigee
Copy link
Contributor Author

No worries. I was a bit quick to submit it too. I marked it as 'draft' just to get some more eyes on it.

@rossigee
Copy link
Contributor Author

It looks to me like Harbor does supply a tag as expected in most cases, so this should work fine in my use case (and probably most others too). The 'digest in the tag field' problem shouldn't affect us, it was just a bad example I copied and pasted from their docs, which highlighted the bug you then reported 😄

Anyway, here's an actual real-world example (only app name/namespace changed) that seems legit:

{
   "event_data" : {
      "repository" : {
         "date_created" : 1707912357,
         "name" : "myapp",
         "namespace" : "mynamespace",
         "repo_full_name" : "mynamespace/myapp",
         "repo_type" : "private"
      },
      "resources" : [
         {
            "digest" : "sha256:fd49e9a1a36d28c54520d89bb23f07365802c5c75108c7eaca4400ed60bb19a5",
            "resource_url" : "harbor.mydomain.lan/mynamespace/myapp:master-491-6bac849",
            "tag" : "master-491-6bac849"
         }
      ]
   },
   "occur_at" : 1713095086,
   "operator" : "build-myapp",
   "type" : "PUSH_ARTIFACT"
}

@bluebrown
Copy link
Owner

No worries. I was a bit quick to submit it too. I marked it as 'draft' just to get some more eyes on it.

Lets learn from that and take it a tad more slow next time. I like it being tagged as draft so I dont rush.

Regarding your example, so you think its fine like that? As far as I understood from the issue, this happens when you use some cache-proxy feature. When the proxy pulls the image from upstream, it triggers a push event downstream. This event will put the digest and not the tag.

If its ok like this, then I think it should be still as in the new linked PR. The other decoders try to add the digest to the ref. like img:tag@digest. So kobold can match by tag but pin by digest. Because upstream registries usally ignore tags when there is also a digest.

@rossigee
Copy link
Contributor Author

I can now confirm #53 works as expected with Harbor webhooks.

@bluebrown
Copy link
Owner

Hi @rossigee , I thought about the other one. Both of them sidestep the digest, which seems to be aligned with your use case.

But I would like to support using digest and even tag@digest for informational purposes.

@rossigee
Copy link
Contributor Author

rossigee commented Oct 8, 2024

Regarding your example, so you think its fine like that? As far as I understood from the issue, this happens when you use some cache-proxy feature. When the proxy pulls the image from upstream, it triggers a push event downstream. This event will put the digest and not the tag.

Sorry, I'm confused. For me, the issue was simply that Kobold didn't support Harbor-style webhooks. What proxy are we talking about here?

If its ok like this, then I think it should be still as in the new linked PR. The other decoders try to add the digest to the ref. like img:tag@digest. So kobold can match by tag but pin by digest. Because upstream registries usally ignore tags when there is also a digest.

I'm struggling to understand your use case here. In other words, I'm not quite grokking why you'd need to use kobold at all if you're trying to pin by digest 🤷

As for this ticket, given Kobold supports Harbor now (at least, for most cases I can foresee), and the related PR has been merged/released, so I'd say it's done 🎆 Cheers @bluebrown 🙏

Any remaining use cases that haven't been covered are probably best put in another ticket specifically for that, so we can try to better articulate, understand and discuss such cases and get them addressed in an upcoming release.

@rossigee rossigee closed this as completed Oct 8, 2024
@bluebrown
Copy link
Owner

bluebrown commented Oct 8, 2024

@rossigee , The digest is useful if you are planning to push tags more than once.

For example, someone keeps pushing "dev" or "nightly". It can become problematic if you only pin by tag. You can never be quite sure what version of "nightly" you are actually looking at.

Registries will keep previous digests for the same tag. Otherwise, digest pinning could never work.

Kobold makes use of those dangling digests. While the user uses tag semantics to control kobold, kobold will append the digest to the image refs it finds.

For the user nothing changes, really. But you get some safety hooks for free kind of. Again, confusing images with one another, isnt fun. It can also be k8s that is the confused one sometimes.

Now if you are a good container citizen and have ecxcluvely unique tags, then this has no impact on you wahtsoever. That doesnt meant there isnt another harbor user who would be affected, though.

@bluebrown
Copy link
Owner

One more thing,

It does help to trigger your rollout. In case you push the same tag twice. Because otherwise kubernetes would only see that the podspec hasnt changed and nothing would happen.

@bluebrown
Copy link
Owner

Here something like this. The idea isnt novel. https://github.com/google/k8s-digester

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants