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

feat(auth): allow optional basic authentication (backport #95) #96

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Oct 25, 2023

This is an automatic backport of pull request #95 done by Mergify.


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@mergify mergify bot assigned tthvo Oct 25, 2023
@ebaron ebaron added safe-to-test feat New feature or request labels Oct 25, 2023
@ebaron
Copy link
Member

ebaron commented Oct 25, 2023

@tthvo CI is failing for this one. Only the backport strangely. It seems to be upgrade related, but I'm having trouble pinpointing the cause in the logs. If it is upgrade related, it could be something similar to #82 (comment).

@tthvo
Copy link
Member

tthvo commented Oct 25, 2023

Ahh hmm, I remember handling that null case in...36ff259 (in #95).

I am not seeing Error: UPGRADE FAILED: ... in logs as before. But i saw these:

2023-10-25T23:07:16.3718485Z 72s Warning FailedScheduling pod/cryostat-v4lou3pxqe-85c5dccd49-h6r8n default-scheduler 0/1 nodes are available: 1 node(s) had untolerated taint {node.kubernetes.io/not-ready: }. preemption: 0/1 nodes are available: 1 Preemption is not helpful for scheduling.. 72s 1 cryostat-v4lou3pxqe-85c5dccd49-h6r8n.17917c439e48d520

Error: pod cryostat-v4lou3pxqe-test-connection failed
Upgrade testing for release "cryostat-v4lou3pxqe" skipped because of previous revision testing error: failed waiting for process: exit status 1

Might just be kind issue?

@tthvo
Copy link
Member

tthvo commented Oct 26, 2023

Ahh hahaha oh my. I figured it was because there is a mismatched between Chart.yaml and the core image AppVersion.

  • On this branch, Chart.yaml has 2.4.0-dev as AppVersion while the core is latest (which is 2.5.0-dev.
  • The test checks for this AppVersion as part of the connection tests>

jq -e '{{ printf "(.cryostatVersion | test(\"^v%s\")) and .datasourceAvailable == %t" (.Chart.AppVersion | squote) (not .Values.minimal) }}' /tmp/out.json;

This patch will make the test pass:

diff --git a/charts/cryostat/Chart.yaml b/charts/cryostat/Chart.yaml
index 49a8052..0a4c97a 100644
--- a/charts/cryostat/Chart.yaml
+++ b/charts/cryostat/Chart.yaml
@@ -8,7 +8,7 @@ version: "0.4.0-dev"
 
 kubeVersion: ">= 1.19.0-0"
 
-appVersion: "2.4.0-dev"
+appVersion: "2.5.0-dev"
 
 home: "https://cryostat.io"

@tthvo
Copy link
Member

tthvo commented Oct 26, 2023

I guess we should update the tags of images in values.yaml to 2.4.0 release.

@ebaron
Copy link
Member

ebaron commented Oct 26, 2023

Ah, that explains it! I think we need to add some steps to do this for the operator and Helm Chart when branching for release.

@tthvo
Copy link
Member

tthvo commented Oct 26, 2023

Sounds good! I might have missed that when documenting these steps haha^^

@ebaron
Copy link
Member

ebaron commented Oct 27, 2023

@Mergifyio rebase

@mergify
Copy link
Contributor Author

mergify bot commented Oct 27, 2023

rebase

❌ Unable to rebase: user ebaron is unknown.

Please make sure ebaron has logged in Mergify dashboard.

@ebaron
Copy link
Member

ebaron commented Oct 27, 2023

@Mergifyio rebase

@mergify
Copy link
Contributor Author

mergify bot commented Oct 27, 2023

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/1)
Auto-merging charts/cryostat/README.md
CONFLICT (content): Merge conflict in charts/cryostat/README.md
Auto-merging charts/cryostat/values.schema.json
Auto-merging charts/cryostat/values.yaml
error: could not apply 5ad3810... feat(auth): allow optional basic authentication (#95)
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 5ad3810... feat(auth): allow optional basic authentication (#95)

* feat(values): add authentication value parameters

* feat(auth): implement basic authentication

* fix(values): handle nullable cases

* docs(readme): update to mention unset auth case

(cherry picked from commit e6be8ea)
@ebaron ebaron force-pushed the mergify/bp/cryostat-v2.4/pr-95 branch from 0f5a9c9 to fbde6b6 Compare October 27, 2023 17:42
@ebaron
Copy link
Member

ebaron commented Oct 27, 2023

Finally got it fixed!

@ebaron ebaron merged commit 111ccfd into cryostat-v2.4 Oct 27, 2023
4 checks passed
@mergify mergify bot deleted the mergify/bp/cryostat-v2.4/pr-95 branch October 27, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants