Skip to content

Commit

Permalink
remove need for internal k8s service hosts in provided cert/key pair
Browse files Browse the repository at this point in the history
  • Loading branch information
alecmerdler committed Mar 25, 2021
1 parent 83404ca commit 1952296
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 102 deletions.
1 change: 1 addition & 0 deletions apis/quay/v1/quayregistry_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ var allComponents = []ComponentKind{
var requiredComponents = []ComponentKind{
ComponentPostgres,
ComponentObjectStorage,
ComponentRoute,
}

// QuayRegistrySpec defines the desired state of QuayRegistry.
Expand Down
4 changes: 3 additions & 1 deletion controllers/quay/quayregistry_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (r *QuayRegistryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
updatedQuay.Status.Conditions = v1.RemoveCondition(updatedQuay.Status.Conditions, v1.ConditionTypeRolloutBlocked)

for _, component := range updatedQuay.Spec.Components {
contains, err := kustomize.ContainsComponentConfig(userProvidedConfig, component.Kind)
contains, err := kustomize.ContainsComponentConfig(userProvidedConfig, component)
if err != nil {
updatedQuay, err = r.updateWithCondition(&quay, v1.ConditionTypeRolloutBlocked, metav1.ConditionTrue, v1.ConditionReasonConfigInvalid, err.Error())
if err != nil {
Expand Down Expand Up @@ -463,6 +463,8 @@ func (r *QuayRegistryReconciler) createOrUpdateObject(ctx context.Context, obj k

immutableResources := map[string]bool{
schema.GroupVersionKind{Group: "batch", Version: "v1", Kind: "Job"}.String(): true,
// FIXME(alecmerdler): Probably need to add `Service` here
// schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}.String(): true,
}

log := r.Log.WithValues(
Expand Down
2 changes: 1 addition & 1 deletion kustomize/base/quay.service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ metadata:
labels:
quay-component: quay-app
spec:
type: LoadBalancer
type: ClusterIP
ports:
- protocol: TCP
name: https
Expand Down
2 changes: 1 addition & 1 deletion pkg/configure/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func ReconfigureHandler(k8sClient client.Client) func(w http.ResponseWriter, r *
// Infer managed/unmanaged components from the given `config.yaml`.
newComponents := []v1.Component{}
for _, component := range quay.Spec.Components {
contains, err := kustomize.ContainsComponentConfig(reconfigureRequest.Config, component.Kind)
contains, err := kustomize.ContainsComponentConfig(reconfigureRequest.Config, component)

if err != nil {
log.Error(err, "failed to check `config.yaml` for component fieldgroup", "component", component.Kind)
Expand Down
35 changes: 18 additions & 17 deletions pkg/kustomize/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ func BaseConfig() map[string]interface{} {
}

// EnsureTLSFor checks if given TLS cert/key pair are valid for the Quay registry to use for secure communication with clients,
// and generates a TLS certificate/key pair if they are invalid.
// In addition to `SERVER_HOSTNAME`, it sets certificate subject alternative names
// for the internal k8s service hostnames (i.e. `registry-quay-app.quay-enterprise.svc`).
// and generates a TLS certificate/key pair if they are not provided.
func EnsureTLSFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, tlsCert, tlsKey []byte) ([]byte, []byte, error) {
fieldGroup, err := FieldGroupFor(ctx, "route", quay)
if err != nil {
Expand All @@ -169,23 +167,22 @@ func EnsureTLSFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, t

routeFieldGroup := fieldGroup.(*hostsettings.HostSettingsFieldGroup)

svc := quay.GetName() + "-quay-app"
hosts := []string{
routeFieldGroup.ServerHostname,
svc,
strings.Join([]string{svc, quay.GetNamespace(), "svc"}, "."),
strings.Join([]string{svc, quay.GetNamespace(), "svc", "cluster", "local"}, "."),
}

// Only add BUILDMAN_HOSTNAME as host if provided.
if ctx.BuildManagerHostname != "" {
hosts = append(hosts, strings.Split(ctx.BuildManagerHostname, ":")[0])
}

if tlsCert == nil && tlsKey == nil {
return cert.GenerateSelfSignedCertKey(routeFieldGroup.ServerHostname, []net.IP{}, hosts)
}

for _, host := range hosts {
if valid, _ := shared.ValidateCertPairWithHostname(tlsCert, tlsKey, host, fieldGroupNameFor("route")); !valid {
fmt.Printf("Host %s not valid for certificates provided. Generating self-signed certs", host) // change to logger?
return cert.GenerateSelfSignedCertKey(routeFieldGroup.ServerHostname, []net.IP{}, hosts)
if valid, validationErr := shared.ValidateCertPairWithHostname(tlsCert, tlsKey, host, fieldGroupNameFor("route")); !valid {
return nil, nil, fmt.Errorf("provided certificate/key pair not valid for host '%s': %s", host, validationErr.String())
}
}

Expand All @@ -195,10 +192,10 @@ func EnsureTLSFor(ctx *quaycontext.QuayRegistryContext, quay *v1.QuayRegistry, t
// ContainsComponentConfig accepts a full `config.yaml` and determines if it contains
// the fieldgroup for the given component by comparing it with the fieldgroup defaults.
// TODO: Replace this with function from `config-tool` library once implemented.
func ContainsComponentConfig(fullConfig map[string]interface{}, component v1.ComponentKind) (bool, error) {
func ContainsComponentConfig(fullConfig map[string]interface{}, component v1.Component) (bool, error) {
fields := []string{}

switch component {
switch component.Kind {
case v1.ComponentClair:
fields = (&securityscanner.SecurityScannerFieldGroup{}).Fields()
case v1.ComponentPostgres:
Expand All @@ -213,16 +210,20 @@ func ContainsComponentConfig(fullConfig map[string]interface{}, component v1.Com
case v1.ComponentMirror:
fields = (&repomirror.RepoMirrorFieldGroup{}).Fields()
case v1.ComponentRoute:
for _, field := range (&hostsettings.HostSettingsFieldGroup{}).Fields() {
// SERVER_HOSTNAME is a special field which we allow when using managed `route` component.
if field != "SERVER_HOSTNAME" {
fields = append(fields, field)
fields = (&hostsettings.HostSettingsFieldGroup{}).Fields()

if component.Managed {
for i, field := range fields {
// SERVER_HOSTNAME is a special field which we allow when using managed `route` component.
if field == "SERVER_HOSTNAME" {
fields = append(fields[:i], fields[i+1:]...)
}
}
}
case v1.ComponentMonitoring:
return false, nil
default:
panic("unknown component: " + component)
panic("unknown component: " + component.Kind)
}

// FIXME: Only checking for the existance of a single field
Expand Down
Loading

0 comments on commit 1952296

Please sign in to comment.