Skip to content

Commit

Permalink
Reuse gangway client secret if present (#980)
Browse files Browse the repository at this point in the history 
* Reuse gangway client secret if present
* Apply suggestions from code review
* Add one more unit test

Co-Authored-By: David Ko <[email protected]>
Signed-off-by: JenTing Hsiao <[email protected]>

(cherry picked from commit 1b6eaa0)
Signed-off-by: Danny Sauer <[email protected]>
  • Loading branch information
@innobead
2 people authored and Danny Sauer committed Mar 2, 2020
1 parent 3362b53 commit 6e4bad3
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 12 deletions.
16 changes: 14 additions & 2 deletions internal/pkg/skuba/addons/dex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@
package addons

import (
"github.com/pkg/errors"
"k8s.io/kubernetes/cmd/kubeadm/app/images"

"github.com/SUSE/skuba/internal/pkg/skuba/dex"
"github.com/SUSE/skuba/internal/pkg/skuba/gangway"
"github.com/SUSE/skuba/internal/pkg/skuba/kubernetes"
"github.com/SUSE/skuba/internal/pkg/skuba/skuba"
skubaconstants "github.com/SUSE/skuba/pkg/skuba"
"github.com/pkg/errors"
"k8s.io/kubernetes/cmd/kubeadm/app/images"
)

var gangwayClientSecret string
Expand Down Expand Up @@ -54,10 +56,20 @@ func renderDexTemplate(addonConfiguration AddonConfiguration) string {
type dexCallbacks struct{}

func (dexCallbacks) beforeApply(addonConfiguration AddonConfiguration, skubaConfiguration *skuba.SkubaConfiguration) error {
var err error

client, err := kubernetes.GetAdminClientSet()
if err != nil {
return errors.Wrap(err, "could not get admin client set")
}

// Read gangway client secret from secret resource if present
// in order to update global variable gangwayClientSecret.
gangwayClientSecret, err = gangway.GetClientSecret(client)
if err != nil {
return errors.Wrap(err, "unable to determine if gangway client secret exists")
}

dexCertExists, err := dex.DexCertExists(client)
if err != nil {
return errors.Wrap(err, "unable to determine if dex certificate exists")
Expand Down
13 changes: 11 additions & 2 deletions internal/pkg/skuba/addons/gangway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
package addons

import (
"github.com/pkg/errors"
"k8s.io/kubernetes/cmd/kubeadm/app/images"

"github.com/SUSE/skuba/internal/pkg/skuba/gangway"
"github.com/SUSE/skuba/internal/pkg/skuba/kubernetes"
"github.com/SUSE/skuba/internal/pkg/skuba/skuba"
skubaconstants "github.com/SUSE/skuba/pkg/skuba"
"github.com/pkg/errors"
"k8s.io/kubernetes/cmd/kubeadm/app/images"
)

func init() {
Expand All @@ -49,6 +50,14 @@ func (gangwayCallbacks) beforeApply(addonConfiguration AddonConfiguration, skuba
if err != nil {
return errors.Wrap(err, "could not get admin client set")
}

// Read gangway client secret from secret resource if present
// in order to update global variable gangwayClientSecret.
gangwayClientSecret, err = gangway.GetClientSecret(client)
if err != nil {
return errors.Wrap(err, "unable to determine if gangway client secret exists")
}

gangwaySecretExists, err := gangway.GangwaySecretExists(client)
if err != nil {
return errors.Wrap(err, "unable to determine if gangway secret exists")
Expand Down
30 changes: 30 additions & 0 deletions internal/pkg/skuba/gangway/gangway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import (

"github.com/pkg/errors"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient"
"k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
"sigs.k8s.io/yaml"

"github.com/SUSE/skuba/internal/pkg/skuba/kubernetes"
"github.com/SUSE/skuba/internal/pkg/skuba/node"
Expand All @@ -41,6 +43,8 @@ const (

sessionKey = "session-key"
secretKeyName = "oidc-gangway-secret"

configmapName = "oidc-gangway-config"
)

// GenerateSessionKey generates session key
Expand Down Expand Up @@ -73,6 +77,32 @@ func CreateOrUpdateSessionKeyToSecret(client clientset.Interface, key []byte) er
return nil
}

// GetClientSecret returns the client secret from configmap
func GetClientSecret(client clientset.Interface) (string, error) {
type config struct {
ClientSecret string `yaml:"clientSecret"`
}

cm, err := client.CoreV1().ConfigMaps(metav1.NamespaceSystem).Get(configmapName, metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return "", nil
}
return "", err
}

data, ok := cm.Data["gangway.yaml"]
if !ok {
return "", nil
}

c := config{}
if err := yaml.Unmarshal([]byte(data), &c); err != nil {
return "", err
}
return c.ClientSecret, nil
}

// CreateCert creates a signed certificate for gangway
// with kubernetes CA certificate and key
func CreateCert(client clientset.Interface, pkiPath, kubeadmInitConfPath string) error {
Expand Down
100 changes: 94 additions & 6 deletions internal/pkg/skuba/gangway/gangway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"k8s.io/client-go/kubernetes/fake"
)

func Test_GenerateSessionKey(t *testing.T) {
func TestGenerateSessionKey(t *testing.T) {
tests := []struct {
name string
inputLen int
Expand Down Expand Up @@ -61,7 +61,7 @@ func Test_GenerateSessionKey(t *testing.T) {
}
}

func Test_CreateOrUpdateSessionKeyToSecret(t *testing.T) {
func TestCreateOrUpdateSessionKeyToSecret(t *testing.T) {
tests := []struct {
name string
key []byte
Expand All @@ -83,7 +83,95 @@ func Test_CreateOrUpdateSessionKeyToSecret(t *testing.T) {
}
}

func Test_CreateCert(t *testing.T) {
func TestGetClientSecret(t *testing.T) {
manifest := `
clusterName: example
redirectURL: "https://example.com/callback"
scopes: ["openid", "email", "groups", "profile", "offline_access"]
serveTLS: true
authorizeURL: "https://example.com:32000/auth"
tokenURL: "https://example.com:32000/token"
keyFile: /etc/gangway/pki/tls.key
certFile: /etc/gangway/pki/tls.crt
clientID: "oidc"
clientSecret: "someClientSecret"
usernameClaim: "email"
apiServerURL: "https://example.com:6443"
cluster_ca_path: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
trustedCAPath: /etc/gangway/pki/ca.crt
customHTMLTemplatesDir: /usr/share/caasp-gangway/web/templates/caasp
`

tests := []struct {
name string
client clientset.Interface
expectedError bool
expectClientSecret string
}{
{
name: "get client secret success",
client: fake.NewSimpleClientset(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configmapName,
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
"gangway.yaml": manifest,
},
}),
expectClientSecret: "someClientSecret",
},
{
name: "client secret not exist",
client: fake.NewSimpleClientset(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configmapName,
Namespace: metav1.NamespaceDefault,
},
Data: map[string]string{
"gangway.yaml": manifest,
},
}),
expectClientSecret: "",
},
{
name: "client secret key not exist",
client: fake.NewSimpleClientset(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: configmapName,
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
"ooxx.yaml": manifest,
},
}),
expectClientSecret: "",
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
gotClientSecret, err := GetClientSecret(tt.client)
if tt.expectedError {
if err == nil {
t.Errorf("error expected on %s, but no error reported", tt.name)
}
return
} else if err != nil {
t.Errorf("error not expected on %s, but an error was reported (%v)", tt.name, err)
return
}

if gotClientSecret != tt.expectClientSecret {
t.Errorf("got %s, want %s", gotClientSecret, tt.expectClientSecret)
return
}
})
}
}

func TestCreateCert(t *testing.T) {
tests := []struct {
name string
pkiPath string
Expand Down Expand Up @@ -124,7 +212,7 @@ func Test_CreateCert(t *testing.T) {
}
}

func Test_GangwaySecretExists(t *testing.T) {
func TestGangwaySecretExists(t *testing.T) {
tests := []struct {
name string
client clientset.Interface
Expand Down Expand Up @@ -171,7 +259,7 @@ func Test_GangwaySecretExists(t *testing.T) {
}
}

func Test_GangwayCertExists(t *testing.T) {
func TestGangwayCertExists(t *testing.T) {
tests := []struct {
name string
client clientset.Interface
Expand Down Expand Up @@ -218,7 +306,7 @@ func Test_GangwayCertExists(t *testing.T) {
}
}

func Test_RestartPods(t *testing.T) {
func TestRestartPods(t *testing.T) {
tests := []struct {
name string
client clientset.Interface
Expand Down
4 changes: 2 additions & 2 deletions vendor/modules.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ k8s.io/apiextensions-apiserver/pkg/features
k8s.io/apimachinery/pkg/util/version
k8s.io/apimachinery/pkg/apis/meta/v1
k8s.io/apimachinery/pkg/types
k8s.io/apimachinery/pkg/runtime
k8s.io/apimachinery/pkg/api/errors
k8s.io/apimachinery/pkg/runtime
k8s.io/apimachinery/pkg/util/wait
k8s.io/apimachinery/pkg/runtime/schema
k8s.io/apimachinery/pkg/runtime/serializer
Expand All @@ -234,11 +234,11 @@ k8s.io/apimachinery/pkg/selection
k8s.io/apimachinery/pkg/util/runtime
k8s.io/apimachinery/pkg/watch
k8s.io/apimachinery/pkg/util/strategicpatch
k8s.io/apimachinery/pkg/util/validation/field
k8s.io/apimachinery/pkg/conversion/queryparams
k8s.io/apimachinery/pkg/util/json
k8s.io/apimachinery/pkg/util/naming
k8s.io/apimachinery/pkg/util/sets
k8s.io/apimachinery/pkg/util/validation/field
k8s.io/apimachinery/pkg/runtime/serializer/json
k8s.io/apimachinery/pkg/runtime/serializer/versioning
k8s.io/apimachinery/pkg/api/meta
Expand Down

0 comments on commit 6e4bad3

Please sign in to comment.