Avoid exposing sensitive data to client (#1543)

This PR changes the behavior of controller API which
wrongly exposes sensitive data to the client. Now, the
API only returns success if secret/configmap exists;
otherwise, an error will be returned.
This commit is contained in:
Ta-Ching Chen
2020-02-24 20:02:33 +08:00
committed by GitHub
parent fc5711ce7c
commit 5c09099084
8 changed files with 27 additions and 67 deletions
+2 -2
View File
@@ -239,8 +239,8 @@ func (api *API) GetHandler() http.Handler {
r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiUpdate).Methods("PUT") r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiUpdate).Methods("PUT")
r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiDelete).Methods("DELETE") r.HandleFunc("/v2/triggers/messagequeue/{mqTrigger}", api.MessageQueueTriggerApiDelete).Methods("DELETE")
r.HandleFunc("/v2/secrets/{secret}", api.SecretGet).Methods("GET") r.HandleFunc("/v2/secrets/{secret}", api.SecretExists).Methods("GET")
r.HandleFunc("/v2/configmaps/{configmap}", api.ConfigMapGet).Methods("GET") r.HandleFunc("/v2/configmaps/{configmap}", api.ConfigMapExists).Methods("GET")
r.HandleFunc("/v2/canaryconfigs", api.CanaryConfigApiCreate).Methods("POST") r.HandleFunc("/v2/canaryconfigs", api.CanaryConfigApiCreate).Methods("POST")
r.HandleFunc("/v2/canaryconfigs/{canaryConfig}", api.CanaryConfigApiGet).Methods("GET") r.HandleFunc("/v2/canaryconfigs/{canaryConfig}", api.CanaryConfigApiGet).Methods("GET")
+5 -6
View File
@@ -17,11 +17,10 @@ limitations under the License.
package fake package fake
import ( import (
v1 "github.com/fission/fission/pkg/controller/client/v1"
"io" "io"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "github.com/fission/fission/pkg/controller/client/v1"
"github.com/fission/fission/pkg/info" "github.com/fission/fission/pkg/info"
) )
@@ -34,12 +33,12 @@ func newMiscClient(c *v1.V1) v1.MiscInterface {
return &FakeMisc{} return &FakeMisc{}
} }
func (c *FakeMisc) SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) { func (c *FakeMisc) SecretExists(m *metav1.ObjectMeta) error {
return nil, nil return nil
} }
func (c *FakeMisc) ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) { func (c *FakeMisc) ConfigMapExists(m *metav1.ObjectMeta) error {
return nil, nil return nil
} }
func (c *FakeMisc) GetSvcURL(label string) (string, error) { func (c *FakeMisc) GetSvcURL(label string) (string, error) {
+8 -33
View File
@@ -24,7 +24,6 @@ import (
"net/http" "net/http"
"github.com/pkg/errors" "github.com/pkg/errors"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/fission/fission/pkg/controller/client/rest" "github.com/fission/fission/pkg/controller/client/rest"
@@ -39,8 +38,8 @@ type (
} }
MiscInterface interface { MiscInterface interface {
SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) SecretExists(m *metav1.ObjectMeta) error
ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) ConfigMapExists(m *metav1.ObjectMeta) error
GetSvcURL(label string) (string, error) GetSvcURL(label string) (string, error)
ServerInfo() (*info.ServerInfo, error) ServerInfo() (*info.ServerInfo, error)
PodLogs(m *metav1.ObjectMeta) (io.ReadCloser, int, error) PodLogs(m *metav1.ObjectMeta) (io.ReadCloser, int, error)
@@ -55,52 +54,28 @@ func newMiscClient(c *V1) MiscInterface {
return &Misc{client: c.restClient} return &Misc{client: c.restClient}
} }
func (c *Misc) SecretGet(m *metav1.ObjectMeta) (*apiv1.Secret, error) { func (c *Misc) SecretExists(m *metav1.ObjectMeta) error {
relativeUrl := fmt.Sprintf("secrets/%v", m.Name) relativeUrl := fmt.Sprintf("secrets/%v", m.Name)
relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace) relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace)
resp, err := c.client.Get(relativeUrl) resp, err := c.client.Get(relativeUrl)
if err != nil { if err != nil {
return nil, err return err
} }
defer resp.Body.Close() defer resp.Body.Close()
return nil
body, err := handleResponse(resp)
if err != nil {
return nil, err
}
var secret apiv1.Secret
err = json.Unmarshal(body, &secret)
if err != nil {
return nil, err
}
return &secret, nil
} }
func (c *Misc) ConfigMapGet(m *metav1.ObjectMeta) (*apiv1.ConfigMap, error) { func (c *Misc) ConfigMapExists(m *metav1.ObjectMeta) error {
relativeUrl := fmt.Sprintf("configmaps/%v", m.Name) relativeUrl := fmt.Sprintf("configmaps/%v", m.Name)
relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace) relativeUrl += fmt.Sprintf("?namespace=%v", m.Namespace)
resp, err := c.client.Get(relativeUrl) resp, err := c.client.Get(relativeUrl)
if err != nil { if err != nil {
return nil, err return err
} }
defer resp.Body.Close() defer resp.Body.Close()
return nil
body, err := handleResponse(resp)
if err != nil {
return nil, err
}
var configMap apiv1.ConfigMap
err = json.Unmarshal(body, &configMap)
if err != nil {
return nil, err
}
return &configMap, nil
} }
func (c *Misc) GetSvcURL(label string) (string, error) { func (c *Misc) GetSvcURL(label string) (string, error) {
+3 -10
View File
@@ -17,7 +17,6 @@ limitations under the License.
package controller package controller
import ( import (
"encoding/json"
"net/http" "net/http"
"github.com/gorilla/mux" "github.com/gorilla/mux"
@@ -25,7 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func (a *API) ConfigMapGet(w http.ResponseWriter, r *http.Request) { func (a *API) ConfigMapExists(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r) vars := mux.Vars(r)
name := vars["configmap"] name := vars["configmap"]
ns := a.extractQueryParamFromRequest(r, "namespace") ns := a.extractQueryParamFromRequest(r, "namespace")
@@ -33,17 +32,11 @@ func (a *API) ConfigMapGet(w http.ResponseWriter, r *http.Request) {
ns = metav1.NamespaceDefault ns = metav1.NamespaceDefault
} }
configMap, err := a.kubernetesClient.CoreV1().ConfigMaps(ns).Get(name, metav1.GetOptions{}) _, err := a.kubernetesClient.CoreV1().ConfigMaps(ns).Get(name, metav1.GetOptions{})
if err != nil { if err != nil {
a.logger.Error("error getting config map", zap.Error(err), zap.String("config_map_name", name), zap.String("namespace", ns)) a.logger.Error("error getting config map", zap.Error(err), zap.String("config_map_name", name), zap.String("namespace", ns))
a.respondWithError(w, err) a.respondWithError(w, err)
return return
} }
a.respondWithSuccess(w, nil)
resp, err := json.Marshal(configMap)
if err != nil {
a.respondWithError(w, err)
return
}
a.respondWithSuccess(w, resp)
} }
+3 -10
View File
@@ -17,7 +17,6 @@ limitations under the License.
package controller package controller
import ( import (
"encoding/json"
"net/http" "net/http"
"github.com/gorilla/mux" "github.com/gorilla/mux"
@@ -25,7 +24,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
) )
func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) { func (a *API) SecretExists(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r) vars := mux.Vars(r)
name := vars["secret"] name := vars["secret"]
ns := a.extractQueryParamFromRequest(r, "namespace") ns := a.extractQueryParamFromRequest(r, "namespace")
@@ -33,7 +32,7 @@ func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) {
ns = metav1.NamespaceDefault ns = metav1.NamespaceDefault
} }
secret, err := a.kubernetesClient.CoreV1().Secrets(ns).Get(name, metav1.GetOptions{}) _, err := a.kubernetesClient.CoreV1().Secrets(ns).Get(name, metav1.GetOptions{})
if err != nil { if err != nil {
a.logger.Error("error getting secret", a.logger.Error("error getting secret",
zap.Error(err), zap.Error(err),
@@ -42,11 +41,5 @@ func (a *API) SecretGet(w http.ResponseWriter, r *http.Request) {
a.respondWithError(w, err) a.respondWithError(w, err)
return return
} }
a.respondWithSuccess(w, nil)
resp, err := json.Marshal(secret)
if err != nil {
a.respondWithError(w, err)
return
}
a.respondWithSuccess(w, resp)
} }
+2 -2
View File
@@ -222,7 +222,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
// check the referenced secret is in the same ns as the function, if not give a warning. // check the referenced secret is in the same ns as the function, if not give a warning.
if !toSpec { // TODO: workaround in order not to block users from creating function spec, remove it. if !toSpec { // TODO: workaround in order not to block users from creating function spec, remove it.
for _, secretName := range secretNames { for _, secretName := range secretNames {
_, err := opts.Client().V1().Misc().SecretGet(&metav1.ObjectMeta{ err := opts.Client().V1().Misc().SecretExists(&metav1.ObjectMeta{
Namespace: fnNamespace, Namespace: fnNamespace,
Name: secretName, Name: secretName,
}) })
@@ -248,7 +248,7 @@ func (opts *CreateSubCommand) complete(input cli.Input) error {
// check the referenced cfgmap is in the same ns as the function, if not give a warning. // check the referenced cfgmap is in the same ns as the function, if not give a warning.
if !toSpec { if !toSpec {
for _, cfgMapName := range cfgMapNames { for _, cfgMapName := range cfgMapNames {
_, err := opts.Client().V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ err := opts.Client().V1().Misc().ConfigMapExists(&metav1.ObjectMeta{
Namespace: fnNamespace, Namespace: fnNamespace,
Name: cfgMapName, Name: cfgMapName,
}) })
+2 -2
View File
@@ -87,7 +87,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
// check that the referenced secret is in the same ns as the function, if not give a warning. // check that the referenced secret is in the same ns as the function, if not give a warning.
for _, secretName := range secretNames { for _, secretName := range secretNames {
_, err := opts.Client().V1().Misc().SecretGet(&metav1.ObjectMeta{ err := opts.Client().V1().Misc().SecretExists(&metav1.ObjectMeta{
Namespace: fnNamespace, Namespace: fnNamespace,
Name: secretName, Name: secretName,
}) })
@@ -111,7 +111,7 @@ func (opts *UpdateSubCommand) complete(input cli.Input) error {
// check that the referenced cfgmap is in the same ns as the function, if not give a warning. // check that the referenced cfgmap is in the same ns as the function, if not give a warning.
for _, cfgMapName := range cfgMapNames { for _, cfgMapName := range cfgMapNames {
_, err := opts.Client().V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ err := opts.Client().V1().Misc().ConfigMapExists(&metav1.ObjectMeta{
Namespace: fnNamespace, Namespace: fnNamespace,
Name: cfgMapName, Name: cfgMapName,
}) })
+2 -2
View File
@@ -400,7 +400,7 @@ func (fr *FissionResources) Validate(input cli.Input) ([]string, error) {
return warnings, err return warnings, err
} }
for _, cm := range f.Spec.ConfigMaps { for _, cm := range f.Spec.ConfigMaps {
_, err := client.V1().Misc().ConfigMapGet(&metav1.ObjectMeta{ err := client.V1().Misc().ConfigMapExists(&metav1.ObjectMeta{
Name: cm.Name, Name: cm.Name,
Namespace: cm.Namespace, Namespace: cm.Namespace,
}) })
@@ -410,7 +410,7 @@ func (fr *FissionResources) Validate(input cli.Input) ([]string, error) {
} }
for _, s := range f.Spec.Secrets { for _, s := range f.Spec.Secrets {
_, err := client.V1().Misc().SecretGet(&metav1.ObjectMeta{ err := client.V1().Misc().SecretExists(&metav1.ObjectMeta{
Name: s.Name, Name: s.Name,
Namespace: s.Namespace, Namespace: s.Namespace,
}) })