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

Display oci helm repositories well #2322

Merged
merged 5 commits into from
Jun 23, 2022
Merged

Display oci helm repositories well #2322

merged 5 commits into from
Jun 23, 2022

Conversation

ozamosi
Copy link
Contributor

@ozamosi ozamosi commented Jun 14, 2022

This upgrades the source controller, exports the helm repository type, and then displays it.

It also disables linking to oci URLs, and handles helm repositories that don't have a last updated time in both the source table and the details page - there's more details why in the UI commit.

End result:

Screenshot from 2022-06-14 18-33-06

Screenshot from 2022-06-14 18-33-20

This closes #2266

@ozamosi ozamosi added the type/enhancement New feature or request label Jun 14, 2022
Copy link
Contributor

@joshri joshri left a comment

Choose a reason for hiding this comment

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

Do you know why the Breadcrumbs and Metadata snapshots changed? Seems like you didn't change anything that would effect those

@opudrovs
Copy link
Contributor

@joshri good catch! Fixed the tests and snapshots.

@ozamosi ozamosi force-pushed the oci-repositories branch 2 times, most recently from d042907 to 34a28f6 Compare June 15, 2022 11:49
Copy link
Contributor

@opudrovs opudrovs left a comment

Choose a reason for hiding this comment

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

The UI part looks good to me, tested it locally. Also looked through the backend part, looks good, but I don't know why the tests are failing.

@@ -45,6 +44,18 @@ function HelmRepositoryDetail({
);
}

function tryLink(url: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a suggestion, maybe we could start adding utility/helper function return types (if not tests). Smth. like React.ReactElement<any, any> | string could work.

@ozamosi ozamosi force-pushed the oci-repositories branch 2 times, most recently from e4e9ee1 to 9b0b0a4 Compare June 22, 2022 12:59
Robin Sonefors added 3 commits June 22, 2022 15:36
This is done to ensure we get the new helm repository type (e.g. oci
or not) passed in.

I did this by `go get -u` which means there's a tonne of other
upgrades included to.

There's one test change, where I _think_ what happened is that a `>=`
changed to `>` and thus the JWT we generated with a 1ms life started
to fail instead of pass. No worries - 500ms should do it, then.
Someone (let's not git blame or point fingers - it could be any of us,
really!) got confused between release and repository, and put the
tests in the wrong file.
This is called "type" in flux -
https://fluxcd.io/docs/components/source/helmrepositories/#type - but
our frontend refers to kind as type, so I'm not taking any chances.

Flux always assumes "default" when it's not "oci", according to
https://github.com/fluxcd/source-controller/blob/812f6e49ddfbe42d5e11da2805b8feb9121cdb88/api/v1beta2/helmrepository_types.go#L34
This does the same thing - it means that if you run an older flux
that doesn't set this field, we will still set it, and set it to a
reasonable value.
@ozamosi ozamosi force-pushed the oci-repositories branch 3 times, most recently from 41e6908 to bee7694 Compare June 22, 2022 14:46
Robin Sonefors and others added 2 commits June 22, 2022 15:50
This
 * Adds the repository type to the detail page, but not to the
   source table, because most pages couldn't have them.
 * Removes hyperlinks from OCI URLs - this string isn't trivial to
   make point to something reasonable, so I decided it's out of
   scope. For example:
   - ghcr.io - this link doesn't work. If you add the chart to the end
     of it, it works, and if you replace ghcr.io with github.com, it
     also works.
   - docker.io - this link doesn't work. If you add the chart, it
     still doesn't work. If you swap docker.io with hub.docker.com/r
     it kinda might sometimes?
   - quay.io - this link works, point at the organisation, all good.
* Removes the last updated time from OCI registries. I was confused
  for a bit about this but I think it makes sense now - lastUpdated
  everywhere we use it refers to the artifact's last update time. For
  a regular helm repository, this is the last time we updated
  index.yaml. For a git repository, it's the last time we git
  pulled. In this case, there's no artifact because there is no
  repository - the "chart" isn't pulled out of a repository, the chart
  _is_ the docker repository and the docker image is the release. So,
  when using oci repositories there's no repository sync at all,
  each chart is synced individually. So there's no last time we synced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPIKE: Behavior of OCI Helm Chart in Weave GitOps
4 participants