Fix/api issues#319
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @nxtcoder17 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| return nil, errors.NewE(err) | ||
| } | ||
|
|
||
| if err := d.syncAccountLevelImagePullSecrets(ctx, nenv.Name, nenv.Spec.TargetNamespace); err != nil { |
There was a problem hiding this comment.
suggestion (code_refinement): Refactoring to a more generic secret sync function
Changing from 'syncAccountLevelImagePullSecrets' to 'syncImagePullSecretsToEnvironment' suggests a move towards a more generic and possibly reusable function. Ensure that this new method handles all specific cases previously covered.
| if err := d.syncAccountLevelImagePullSecrets(ctx, nenv.Name, nenv.Spec.TargetNamespace); err != nil { | |
| if err := d.syncImagePullSecretsToEnvironment(ctx, nenv.Name); err != nil { | |
| return nil, errors.NewE(err) | |
| } |
|
|
||
| data[corev1.DockerConfigJsonKey] = []byte(base64.StdEncoding.EncodeToString(b)) | ||
| // data[corev1.DockerConfigJsonKey] = []byte(base64.StdEncoding.EncodeToString(b)) | ||
| data[corev1.DockerConfigJsonKey] = []byte(b) | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): Change in data encoding for Docker config JSON
The removal of base64 encoding for Docker config JSON data might affect how data is read and interpreted by Kubernetes. Ensure that this change is compatible with how Kubernetes expects and handles this data.
1 participant
No description provided.