-
Notifications
You must be signed in to change notification settings - Fork 104
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
Allow to remap key name when mirroring secret #475
base: main
Are you sure you want to change the base?
Conversation
@winromulus Hi. |
newResource.Metadata ??= new V1ObjectMeta(); | ||
newResource.Metadata.Name = targetId.Name; | ||
newResource.Metadata.NamespaceProperty = targetId.Namespace; | ||
newResource.Metadata.Annotations ??= new Dictionary<string, string>(); | ||
var newResourceAnnotations = newResource.Metadata.Annotations; | ||
foreach (var patchAnnotation in patchAnnotations) | ||
newResourceAnnotations[patchAnnotation.Key] = patchAnnotation.Value; | ||
newResourceAnnotations[Annotations.Reflection.MetaAutoReflects] = autoReflection.ToString(); |
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.
This block is needed for tracing the source. Any reason why it was deleted?
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.
All the affectations are already done by the foreach loop above
README.md
Outdated
|
||
### 2. Annotate the mirror secret or configmap | ||
|
||
- Add `reflector.v1.k8s.emberstack.com/reflects: "<source namespace>/<source name>"` to the mirror object. The value of the annotation is the full name of the source object in `namespace/name` format. | ||
- Optionally Add `reflector.v1.k8s.emberstack.com/reflects: "<source namespace>/<source name>"` to the mirror object. The value of the annotation is the full name of the source object in `namespace/name` format. |
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 think this was supposed to be the key mapping content, since it duplicates the line above.
|
||
Dictionary<string, string> mapping = new Dictionary<string, string>(); | ||
try { mapping = Mapping(patchAnnotations[Annotations.Reflection.KeyMapping]); } |
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.
Please improve the code style. This is extremely hard to read
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.
Not too sure of what you mean here, I assume it is just to add line breaks
2e66842
to
31bfc68
Compare
Use reflector.v1.k8s.emberstack.com/reflection-key-mapping and reflector.v1.k8s.emberstack.com/reflection-auto-key-mapping to do the renaming See README.md for details
{ | ||
foreach (var (key, value) in data) | ||
{ | ||
newData.Add(mapping.GetValueOrDefault(key, key), value); |
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.
mapping.GetValueOrDefault(key, key)
could easily be an empty string - which is fine. But in this case you should not copy the data over.
Example: mapping 'tls.key:' would NOT copy over the private key.
Use case: I have a secret created by the cert manager and I want to copy it over to another namespace. But I only want the public key! Private key must stay with the source...since this is kind of the point of certificates :D
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.
Example: mapping 'tls.key:' would NOT copy over the private key.
Not necessary true, I have a use case where two signer need the same key, I want to copy the key too.
Nevertheless I understand what you mean.
Currently you cannot copy partially, the idea was to improve the functionality, not necessary to add a comprehensive behaviour.
To do something more complex (i.e. what you suggest), a combination reflector + external-secret can do the job at the moment (technically even what I added, but it can be useful for small problems).
Use reflector.v1.k8s.emberstack.com/reflection-key-mapping and reflector.v1.k8s.emberstack.com/reflection-auto-key-mapping to do the renaming
See README.md for details