feat: adds X509 workload cert logic #1527
Conversation
| @@ -63,20 +65,20 @@ def _check_dca_metadata_path(metadata_path): | |||
| return metadata_path | |||
|
|
|||
|
|
|||
| def _read_dca_metadata_file(metadata_path): | |||
| """Loads context aware metadata from the given path. | |||
| def _read_json_file(path): | |||
There was a problem hiding this comment.
I think this function's name is too vague. Can you rename it to provide more context?
There was a problem hiding this comment.
renamed to load Json file, but that's not much better. Not sure what would be more descriptive, this method does simply just read a file and calls Json.load while wrapping in the appropriate error if it fails - open to suggestions
There was a problem hiding this comment.
My preference is something like _load_workload_json_file or something. The code is generic but describing when / what it is used for is useful.
Otherwise it may get shoved in a util file down the line
There was a problem hiding this comment.
With the changes in this PR, this function is used to load json configs for both secure connect as well as workload, hence the generic naming. I think it's fine as is - maybe add a function comment to clarify usage. It does seem more suitable in a generic util file, but I don't have a strong opinion here.
There was a problem hiding this comment.
added another sentence in the comment explaining the method is used for both x509 and secure connect, this definitely could be moved to a util file in the future though, just didn't think that was necessary for this PR.
aeitzman
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
aeitzman
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
aeitzman
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
Adds support for X509 workload logic that reads from the certificate_config.json file. This PR does not actually call that logic yet - hooks for actually using the new logic will come when the credential type is added.