-
Notifications
You must be signed in to change notification settings - Fork 44
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
[#3130] Feature/endpoint for retrieving the current configuration #3177
[#3130] Feature/endpoint for retrieving the current configuration #3177
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.
Maybe you can also add the CLI part that when we do
airy config or airy config get
that the configuration is printed on the CLI.
Default by yaml, maybe if we pass an option airy config -o json
- it prints it in json.
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.
First look. Most importantly docs are still missing
configmapList, err := s.clientSet.CoreV1().ConfigMaps(s.namespace).List(r.Context(), v1.ListOptions{LabelSelector: "core.airy.co/component"}) | ||
if err != nil { | ||
log.Printf("Unable to list config maps. Error: %s\n", err) | ||
w.WriteHeader(http.StatusInternalServerError) |
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.
Do you think it'd be useful to return the error in a json response payload like we do with other endpoints?
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.
When it comes to internal server errors like kubectl errors. they should not be forwarded to the client
|
||
blob, err := json.Marshal(map[string]interface{}{"components": components}) | ||
if err != nil { | ||
log.Printf("Unable to marchal config Error: %s\n", err) |
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.
log.Printf("Unable to marchal config Error: %s\n", err) | |
log.Printf("Unable to marshal config Error: %s\n", err) |
@@ -38,5 +39,8 @@ func Serve(clientSet *kubernetes.Clientset, namespace string) { | |||
s := &Services{clientSet: clientSet, namespace: namespace} | |||
r.Handle("/services", s) | |||
|
|||
cg := &ClusterGet{clientSet: clientSet, namespace: namespace} | |||
r.Handle("/cluster.get", cg).Methods("GET") |
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.
Like all our other endpoints this should be a POST
|
||
var secretMatcher = regexp.MustCompile(`(?i)secret|key|token`) | ||
|
||
func maskSecrets(data map[string]string) map[string]string { |
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.
Very good thinking! Long term I think the services need to add some metadata so that we know which fields to mask without hardcoding it. What do you think?
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.
Yes this is what we were talking yesterday. we can do this in a second iteration
The docs are going to made in #3130 |
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.
Awesome 👍
closes #3130