Skip to content

Added helm chart repo proxy to the console backend #3928

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.

Sign up for GitHub

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

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Jan 21, 2020
Merged

Added helm chart repo proxy to the console backend #3928

openshift-merge-robot merged 1 commit into from
Jan 21, 2020

Conversation

@pedjak
Copy link
Contributor

@pedjak pedjak commented Jan 13, 2020 •
edited
Loading

All requests to /api/helm/charts/* are forwarded to the declared Helm chart repository.
By default this is https://redhat-developer.github.com/redhat-helm-charts, but configurable via

  • helm-chart-repo-url CLI argument
  • BRIDGE_HELM_CHART_REPO_URL env variable
  • config.yaml:
.
.
.
helm:
  chart-repo: 
    url: https://foo.bar

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. component/backend Related to backend labels Jan 13, 2020
spadgett
spadgett requested changes Jan 13, 2020
View reviewed changes
spadgett
spadgett reviewed Jan 13, 2020
View reviewed changes
spadgett
spadgett reviewed Jan 13, 2020
View reviewed changes
@sbose78 sbose78 mentioned this pull request Jan 13, 2020
Merged
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2020
pedjak
pedjak commented Jan 13, 2020
View reviewed changes
@pedjak pedjak force-pushed the helm-chart-repo-proxy branch from 069f676 to c426748 Compare January 14, 2020 00:10
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2020
spadgett
spadgett requested changes Jan 15, 2020
View reviewed changes
spadgett
spadgett reviewed Jan 15, 2020
View reviewed changes
@sbose78
Copy link

sbose78 commented Jan 15, 2020

/retest

benjaminapetersen
benjaminapetersen reviewed Jan 15, 2020
View reviewed changes
cmd/bridge/main.go Outdated
Copy link
Contributor

@benjaminapetersen benjaminapetersen Jan 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Possible to move this logic under your pkg/helm? Something along the lines of:

helmChartRepoURL := helm.chartRepoURL(args)

Ideally main.go should not grow, I'd like it to shrink from now on. :)

benjaminapetersen
benjaminapetersen reviewed Jan 15, 2020
View reviewed changes
@spadgett
Copy link
Member

spadgett commented Jan 15, 2020

I think this is ready, but you need to run gofmt :)

@spadgett spadgett added this to the v4.4 milestone Jan 15, 2020
benjaminapetersen
benjaminapetersen reviewed Jan 15, 2020
View reviewed changes
benjaminapetersen
benjaminapetersen reviewed Jan 15, 2020
View reviewed changes
benjaminapetersen
benjaminapetersen suggested changes Jan 15, 2020
View reviewed changes
Copy link
Contributor

@benjaminapetersen benjaminapetersen left a comment

Choose a reason for hiding this comment

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

Small request, to move a couple lines to avoid growth of main.go if at all possible.

@pedjak
Copy link
Contributor Author

pedjak commented Jan 16, 2020

@benjaminapetersen pushed refactoring

@sbose78
Copy link

sbose78 commented Jan 16, 2020

@pedjak This PR too should be following #3826 (comment) ?

@pedjak
Copy link
Contributor Author

pedjak commented Jan 16, 2020

@sbose78 I would say yes, if I did not miss something.. should I squash commits before, or after getting the green light from @benjaminapetersen ?

@sbose78
Copy link

sbose78 commented Jan 16, 2020

/retest

@sbose78
Copy link

sbose78 commented Jan 16, 2020

Go ahead.
I will defer to @benjaminapetersen for a final lgtm but I would say, your changes look good 👍 .

@pedjak pedjak mentioned this pull request Jan 16, 2020
Merged
benjaminapetersen
benjaminapetersen reviewed Jan 17, 2020
View reviewed changes
@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jan 17, 2020

Tidy up the imports and we should be good to go!

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

22 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 19, 2020

/retest

Please review the full test history for this PR and help us cut down flakes.

@spadgett
Copy link
Member

spadgett commented Jan 19, 2020

/hold
to avoid retest while CI is broken

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2020
@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Jan 20, 2020

/retest

 A Jasmine spec timed out. Resetting the WebDriver Control Flow.

@pedjak
Copy link
Contributor Author

pedjak commented Jan 20, 2020

/retest

@spadgett
Copy link
Member

spadgett commented Jan 20, 2020

/hold cancel
/retest

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2020
@openshift-merge-robot openshift-merge-robot merged commit b6d48ca into openshift:master Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spadgett spadgett spadgett requested changes

@jhadvig jhadvig Awaiting requested review from jhadvig

+2 more reviewers

@benjaminapetersen benjaminapetersen benjaminapetersen requested changes

@sbose78 sbose78 sbose78 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@benjaminapetersen benjaminapetersen

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/backend Related to backend lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v4.4

Development

Successfully merging this pull request may close these issues.

7 participants

@pedjak @sbose78 @spadgett @benjaminapetersen @openshift-ci-robot @openshift-bot @openshift-merge-robot