Skip to content

Commit

Permalink
configobservation: override service-account-jwks-uri to use LB address
Browse files Browse the repository at this point in the history
Without the override, the KAS would point to its own IP which is not
covered by the default serving certificate.
  • Loading branch information
tnozicka authored and stlaz committed Dec 10, 2020
1 parent c7c926f commit 29ac35a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 16 deletions.
34 changes: 26 additions & 8 deletions pkg/operator/configobservation/auth/auth_serviceaccountissuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import (
"net/url"
"strings"

v1 "github.com/openshift/api/config/v1"
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
"k8s.io/klog/v2"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/klog/v2"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/library-go/pkg/operator/configobserver"
"github.com/openshift/library-go/pkg/operator/events"

"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
)

// ObserveServiceAccountIssuer changes apiServerArguments.service-account-issuer from
Expand All @@ -25,19 +27,30 @@ func ObserveServiceAccountIssuer(
) (map[string]interface{}, []error) {

listers := genericListers.(configobservation.Listers)
return observedConfig(existingConfig, listers.AuthConfigLister.Get)
return observedConfig(existingConfig, listers.AuthConfigLister.Get, listers.InfrastructureLister().Get)
}

// observedConfig returns an unstructured fragment of KubeAPIServerConfig that may
// include an override of the default service account issuer if one was set in the
// Authentication resource.
func observedConfig(
existingConfig map[string]interface{},
authConfigAccessor func(string) (*v1.Authentication, error),
authConfigAccessor func(string) (*configv1.Authentication, error),
infrastructureConfigAccessor func(string) (*configv1.Infrastructure, error),
) (map[string]interface{}, []error) {

issuer, errs := observedIssuer(existingConfig, authConfigAccessor)
config := unstructuredConfigForIssuer(issuer)

infrastructureConfig, err := infrastructureConfigAccessor("cluster")
if err != nil {
return existingConfig, append(errs, err)
}
apiServerInternalURL := infrastructureConfig.Status.APIServerInternalURL
if len(apiServerInternalURL) == 0 {
return existingConfig, append(errs, fmt.Errorf("APIServerInternalURL missing from infrastructure/cluster"))
}

config := unstructuredConfigForIssuer(issuer, apiServerInternalURL)
return config, errs
}

Expand All @@ -47,7 +60,7 @@ func observedConfig(
// issuer. If that is not possible, then an empty string will be returned.
func observedIssuer(
existingConfig map[string]interface{},
authConfigAccessor func(string) (*v1.Authentication, error),
authConfigAccessor func(string) (*configv1.Authentication, error),
) (string, []error) {
errs := []error{}

Expand Down Expand Up @@ -105,7 +118,7 @@ func issuerFromUnstructuredConfig(config map[string]interface{}) (string, []erro

// unstructuredConfigForIssuer creates an unstructured KubeAPIServerConfig fragment
// for the given issuer.
func unstructuredConfigForIssuer(issuer string) map[string]interface{} {
func unstructuredConfigForIssuer(issuer, internalLBURL string) map[string]interface{} {
// Returning an empty config when no value is provided for issuer ensures that
// the default value will not be overwritten by an empty value.
if len(issuer) == 0 {
Expand All @@ -116,6 +129,11 @@ func unstructuredConfigForIssuer(issuer string) map[string]interface{} {
"service-account-issuer": []interface{}{
issuer,
},
"service-account-jwks-uri": []interface{}{
// We need to set the URL so that it points to our LB and doesn't
// default to KAS IP which isn't included in the serving certificate.
internalLBURL + "/openid/v1/jwks",
},
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"

apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -19,17 +20,21 @@ import (
func TestObservedConfig(t *testing.T) {
existingConfig := unstructuredAPIConfigForIssuer(t, "https://example.com")
authConfig := authConfigForIssuer("https://example.com")
expectedErr := fmt.Errorf("foo")
expectedErrAuth := fmt.Errorf("foo")
expectedErrInfra := fmt.Errorf("bar")
newConfig, errs := observedConfig(
existingConfig,
func(_ string) (*configv1.Authentication, error) {
return authConfig, expectedErr
return authConfig, expectedErrAuth
},
func(_ string) (*configv1.Infrastructure, error) {
return &configv1.Infrastructure{Status: configv1.InfrastructureStatus{APIServerInternalURL: "https://lb.example.com"}}, expectedErrInfra
},
)

// Check that errors are passed through
require.Len(t, errs, 1)
require.Equal(t, errs[0], expectedErr)
require.Len(t, errs, 2)
require.Equal(t, errs, []error{expectedErrAuth, expectedErrInfra})

// The observed config must unmarshall to
// KubeAPIServerConfig without error.
Expand All @@ -40,8 +45,8 @@ func TestObservedConfig(t *testing.T) {
require.NoError(t, err)
unmarshalledConfig := &kubecontrolplanev1.KubeAPIServerConfig{}
require.NoError(t, json.Unmarshal(jsonConfig, unmarshalledConfig))
expectedConfig := apiConfigForIssuer("https://example.com")
require.Equal(t, expectedConfig, unmarshalledConfig)
expectedConfig := apiConfigForIssuer("https://example.com", "https://lb.example.com")
require.Equal(t, expectedConfig, unmarshalledConfig, cmp.Diff(expectedConfig, unmarshalledConfig))
}

func TestIssuerFromUnstructuredConfig(t *testing.T) {
Expand Down Expand Up @@ -135,12 +140,16 @@ func authConfigForIssuer(issuer string) *configv1.Authentication {
}
}

func apiConfigForIssuer(issuer string) *kubecontrolplanev1.KubeAPIServerConfig {
func apiConfigForIssuer(issuer, lbAddress string) *kubecontrolplanev1.KubeAPIServerConfig {
return &kubecontrolplanev1.KubeAPIServerConfig{
TypeMeta: metav1.TypeMeta{Kind: "KubeAPIServerConfig"},
APIServerArguments: map[string]kubecontrolplanev1.Arguments{
"service-account-issuer": {
issuer,
},
"service-account-jwks-uri": {
lbAddress + "/openid/v1/jwks",
},
},
}
}
Expand All @@ -149,7 +158,7 @@ func apiConfigForIssuer(issuer string) *kubecontrolplanev1.KubeAPIServerConfig {
// to ensure the input to the function under test will match what will
// be received at runtime.
func unstructuredAPIConfigForIssuer(t *testing.T, issuer string) map[string]interface{} {
config := apiConfigForIssuer(issuer)
config := apiConfigForIssuer(issuer, "https://lb.example.com")
// Unmarshaling to unstructured requires explicitly setting kind
config.TypeMeta = metav1.TypeMeta{
Kind: "KubeAPIServerConfig",
Expand Down

0 comments on commit 29ac35a

Please sign in to comment.