[stable/dex] Fix ConfigMap creation and add missed config parameter #18380

lohmag · 2019-10-27T09:49:25Z

Fix for openssl config which used by Job later when hook executed. Becouse of "helm.sh/hook-delete-policy": hook-succeeded Configmap deletes just after creates.
Added ability to set token expire which is realised in dex app config file but not in helm values.yaml
https://github.com/dexidp/dex/blob/master/examples/config-dev.yaml#L38

Is this a new chart

NOTE: We're experiencing a high volume of PRs to this repo and reviews will be delayed. Please host your own chart repository and submit your repository to the Helm Hub instead of this repo to make them discoverable to the community. Here is how to submit new chart repositories to the Helm Hub.

What this PR does / why we need it:

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

fixes #

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
[x ] Chart Version bumped
[x ] Variables are documented in the README.md
[x ] Title of the PR starts with chart name (e.g. [stable/mychartname])

k8s-ci-robot · 2019-10-27T09:49:40Z

Hi @lohmag. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zanhsieh · 2019-10-27T12:31:20Z

@lohmag
Would you mind to sign DCO please?

vi7 · 2019-10-28T15:54:43Z

hi @lohmag , thanks for the contribution. Could you please squash commits into one and name it according to the introduced changes? Also please put a meaningful name for the PR (can be the same as for commit) and prefix it with [stable/dex]

vi7 · 2019-10-28T15:55:50Z

/hold

zanhsieh · 2019-10-29T02:35:34Z

/ok-to-test

lohmag · 2019-10-29T09:31:53Z

/retest

lohmag · 2019-10-29T09:53:47Z

/retest

lohmag · 2019-10-29T09:57:53Z

I don't know how to squash commit into one and can't spend anymore time on it. And actually I don't know why I should do it if you can squash in web interface when it merges by just checking a box.

vi7 · 2019-10-30T12:06:09Z

/hold cancel

Signed-off-by: Nikita Massalitin <[email protected]>

lohmag · 2019-11-02T19:43:18Z

/assign @vi7

desaintmartin · 2019-11-04T09:10:58Z

With this change, we'll have one unused configmap, but it is better than a non-working Chart. Thanks!

Could you bump minor version of this chart instead of patch version since there is a new feature?

/assign

@vi7 FYI, AFAIK, merge bot will squash the commits anyway, so the important thing is the PR title.

lohmag · 2019-11-04T13:26:11Z

With this change, we'll have one unused configmap, but it is better than a non-working Chart. Thanks!

Could you bump minor version of this chart instead of patch version since there is a new feature?

/assign

@vi7 FYI, AFAIK, merge bot will squash the commits anyway, so the important thing is the PR title.

what do you mean by version bumping? i already bumped helm chart version

desaintmartin

bump minor version
Please also document in the README the new value

stable/dex/Chart.yaml

Co-Authored-By: Cédric de Saint Martin <[email protected]> Signed-off-by: Nikita Massalitin <[email protected]>

Signed-off-by: Nikita Massalitin <[email protected]>

vi7 · 2019-11-06T12:00:59Z

@vi7 FYI, AFAIK, merge bot will squash the commits anyway, so the important thing is the PR title.

@desaintmartin I'm ok with that in case merge commit will be named after the PR name, cause current commit names doesn't describe introduced changes and moreover have misleading names

vi7 · 2019-11-06T12:02:02Z

/lgtm
/approve

k8s-ci-robot · 2019-11-06T12:02:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lohmag, vi7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~stable/dex/OWNERS~~ [vi7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

zanhsieh · 2019-11-06T21:23:57Z

/test pull-charts-e2e

…elm#18380) * [stable/traefik] Decouple prometheus metrics to dashboard (helm#14946) Before this PR, in case the dashboard was not enabled, the Prometheus metrics were disabled. It is now possible to enable Prometheus metrics having at the same time the dashboard disabled. Signed-off-by: Salvatore Mazzarino <[email protected]> Fix Fix for Jobs. Jobs also have `"helm.sh/hook": post-install` so they don't have this configmap after start. Pods won't start because of this. Signed-off-by: Nikita Massalitin <[email protected]> Update Chart.yaml version up Signed-off-by: Nikita Massalitin <[email protected]> fix Signed-off-by: Nikita Massalitin <[email protected]> * added expire to values Signed-off-by: Nikita Massalitin <[email protected]> * add ability to set expire Signed-off-by: Nikita Massalitin <[email protected]> * remove blank line Signed-off-by: Nikita Massalitin <[email protected]> * Bump version Co-Authored-By: Cédric de Saint Martin <[email protected]> Signed-off-by: Nikita Massalitin <[email protected]> * Added new conf params to Readme Signed-off-by: Nikita Massalitin <[email protected]>

Since the configmap is needed for the `grpc` and `web` jobs, don't delete it after successful creation. Backported from: helm/charts#18380

helm-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 27, 2019

k8s-ci-robot requested review from desaintmartin and vi7 October 27, 2019 09:49

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 27, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 28, 2019

k8s-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 29, 2019

lohmag changed the title ~~stable/dex~~ [stable/dex] Fix ConfigMap creation Oct 29, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 30, 2019

helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). labels Nov 2, 2019

Nikita Massalitin added 2 commits November 2, 2019 22:06

add ability to set expire

84dde51

Signed-off-by: Nikita Massalitin <[email protected]>

remove blank line

0986c07

Signed-off-by: Nikita Massalitin <[email protected]>

helm-bot added Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 2, 2019

Merge branch 'master' of github.com:helm/charts

7db5d5a

helm-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 2, 2019

k8s-ci-robot assigned vi7 Nov 2, 2019

k8s-ci-robot assigned desaintmartin Nov 4, 2019

desaintmartin suggested changes Nov 4, 2019

View reviewed changes

stable/dex/Chart.yaml Outdated Show resolved Hide resolved

lohmag and others added 2 commits November 4, 2019 20:58

Bump version

e6c12b2

Co-Authored-By: Cédric de Saint Martin <[email protected]> Signed-off-by: Nikita Massalitin <[email protected]>

Added new conf params to Readme

5883c00

Signed-off-by: Nikita Massalitin <[email protected]>

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 6, 2019

k8s-ci-robot merged commit 2c9b6dd into helm:master Nov 6, 2019

jkoelker added a commit to mesosphere/charts that referenced this pull request Jan 9, 2020

dex: keep configmap after creation

1302b73

Since the configmap is needed for the `grpc` and `web` jobs, don't delete it after successful creation. Backported from: helm/charts#18380

jkoelker added a commit to mesosphere/charts that referenced this pull request Jan 10, 2020

dex: keep configmap after creation

738bfcb

Since the configmap is needed for the `grpc` and `web` jobs, don't delete it after successful creation. Backported from: helm/charts#18380

jkoelker added a commit to mesosphere/charts that referenced this pull request Jan 10, 2020

dex: keep configmap after creation

4094e8e

Since the configmap is needed for the `grpc` and `web` jobs, don't delete it after successful creation. Backported from: helm/charts#18380

jkoelker mentioned this pull request Jan 10, 2020

[test] Add chart install and upgrade testing mesosphere/charts#342

Merged

jkoelker added a commit to mesosphere/charts that referenced this pull request Jan 10, 2020

dex: keep configmap after creation

e32c903

Since the configmap is needed for the `grpc` and `web` jobs, don't delete it after successful creation. Backported from: helm/charts#18380

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stable/dex] Fix ConfigMap creation and add missed config parameter #18380

[stable/dex] Fix ConfigMap creation and add missed config parameter #18380

lohmag commented Oct 27, 2019 •

edited

Loading

k8s-ci-robot commented Oct 27, 2019

zanhsieh commented Oct 27, 2019

vi7 commented Oct 28, 2019

vi7 commented Oct 28, 2019

zanhsieh commented Oct 29, 2019

lohmag commented Oct 29, 2019

lohmag commented Oct 29, 2019

lohmag commented Oct 29, 2019

vi7 commented Oct 30, 2019

lohmag commented Nov 2, 2019

desaintmartin commented Nov 4, 2019

lohmag commented Nov 4, 2019

desaintmartin left a comment

vi7 commented Nov 6, 2019

vi7 commented Nov 6, 2019

k8s-ci-robot commented Nov 6, 2019

zanhsieh commented Nov 6, 2019

[stable/dex] Fix ConfigMap creation and add missed config parameter #18380

[stable/dex] Fix ConfigMap creation and add missed config parameter #18380

Conversation

lohmag commented Oct 27, 2019 • edited Loading

Is this a new chart

What this PR does / why we need it:

Which issue this PR fixes

Special notes for your reviewer:

Checklist

k8s-ci-robot commented Oct 27, 2019

zanhsieh commented Oct 27, 2019

vi7 commented Oct 28, 2019

vi7 commented Oct 28, 2019

zanhsieh commented Oct 29, 2019

lohmag commented Oct 29, 2019

lohmag commented Oct 29, 2019

lohmag commented Oct 29, 2019

vi7 commented Oct 30, 2019

lohmag commented Nov 2, 2019

desaintmartin commented Nov 4, 2019

lohmag commented Nov 4, 2019

desaintmartin left a comment

Choose a reason for hiding this comment

vi7 commented Nov 6, 2019

vi7 commented Nov 6, 2019

k8s-ci-robot commented Nov 6, 2019

zanhsieh commented Nov 6, 2019

lohmag commented Oct 27, 2019 •

edited

Loading