PWX-38596 : After disabling PX Security STC goes into DEGRADED state due to rolling update failed #1648
Conversation
nikita-bhatia
force-pushed
the
PWX-38596
branch
from
f8eb161 to
a8b4dee
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-24.2.0 #1648 +/- ##
==================================================
- Coverage 79.79% 79.79% -0.01%
==================================================
Files 61 61
Lines 18834 18831 -3
==================================================
- Hits 15029 15026 -3
Misses 2715 2715
Partials 1090 1090 ☔ View full report in Codecov by Sentry. |
| if errors.IsNotFound(err) { | ||
| if SecurityEnabled(cluster) { | ||
| // when security is enabled in STC, and secret is not found, return error | ||
| return nil, fmt.Errorf("failed to get portworx apps secret: %v", err.Error()) |
There was a problem hiding this comment.
nit: include namespace/name of the secret in the error message
Also, use ("...: %w", err)
There was a problem hiding this comment.
Just wondering why is this secret called apps secret? Shouldn't it just be a security token secret or security context secret?
There was a problem hiding this comment.
Since we are specifically trying to fetch apps-secret key here inside px-system-secrets secret, so the error message is specifying that.
|
If they disable security AND delete the secret, we will still run into an issue? |
Ideally No, because
So to conclude, in a valid cluster state, this would not create problem. |
…due to rolling update failed (#1648) * PWX-38596 : Skip adding token in context if security is disabled in stc * update failing tests
What this PR does / why we need it:
After disabling PX Security STC goes into DEGRADED state due to rolling update failed: cannot upgrade nodes in parallel: failed to get non overlapping nodes: rpc error: code = PermissionDenied desc = Access denied without authentication token
Reason : When security was disabled from previously enabled value, the update on storagenodes is still not done at the point when API call is made to PX to get storage nodes, so even thought security is disabled from STC, storagenode is still not in sync and expects token to be passed by the operator during GRPC call.
Note that `skipSecurityCheck' variable is removed which was previously added in #1539 for improvement of logic as discussed in PR.
Fix
The code change does following to decide if token is required or not to for API calls.
Which issue(s) this PR fixes (optional)
Closes #
https://purestorage.atlassian.net/browse/PWX-38596
https://purestorage.atlassian.net/browse/PWX-37232
Special notes for your reviewer: