-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Change the git-userdata configmap to secret #1313
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vinokurig what about backward compatibility?
Docker image build succeeded: quay.io/eclipse/che-dashboard:pr-1313 kubectl patch commandkubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/dashboard/deployment", "value": {containers: [{image: "quay.io/eclipse/che-dashboard:pr-1313", name: che-dashboard}]}}]" |
We do not have the backward compatibility yet. To have it we can:
WDYT? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1313 +/- ##
==========================================
+ Coverage 91.65% 91.88% +0.23%
==========================================
Files 500 500
Lines 45489 45494 +5
Branches 3173 3187 +14
==========================================
+ Hits 41691 41803 +112
+ Misses 3766 3660 -106
+ Partials 32 31 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vinokurig I tested this PR with Dashboard Version 7.99.0-next.
Let's read the git-userdata(if they exist) from configmap and store it into secret. Then, removed the git-userdata from configmap.
WDYT?
@olexii4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: olexii4, vinokurig The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I faced the same problem, I can't see gitconfig after switching to the new image. |
if (helpers.errors.isKubeClientError(error) && error.statusCode === 404) { | ||
// Create gitconfig configmap if it does not exist | ||
return this.createGitConfigMap(namespace); | ||
// Create gitconfig secret if it does not exist | ||
return this.createGitConfigSecret(namespace); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering how this change would be working from the backward compatibility perspective?
e.g. if there is a cm with gitconfig once the new version of the project is rolled out would the gitconfig be blank?
We decided to close this pull request and go with using the current configmap. |
What does this PR do?
Store the gitconfig automaunt content to secret instead of configmap. This is needed to have all gitconfig automaunt values in one secret as devworkspace operator does not support mount to gitconfig from multiple objects.
The goal of having a single place to mount gitconfig values is that we need to mount additional gitconfig section apart from the
[user]
section. Currently we mount only username and email from theworkspace-userdata-gitconfig-configmap
configmap but in the scope of the Azure devops server support we also need to mount azure token as an http.extraHeader value:We can not mount the
http
section in a separate secret because devworkspace operator does not support gitconfig mount from multiple objects, see https://github.com/devfile/devworkspace-operator/blob/a6ec0bfb254ae1f63283b507475fba69b9768ac5/pkg/provision/automount/common.go#L223-L225.Screenshot/screencast of this PR
What issues does this PR fix or reference?
eclipse-che/che#23306
Is it tested? How?
See: the data is stored to the
devworkspace-gitconfig-automaunt-secret
Secret in the user namespace.Release Notes
Docs PR