Skip to content

Commit

Permalink
MGMT-11456: kube-api should allow user to specify infraenv additional…
Browse files Browse the repository at this point in the history
… trust bundle (#5357)

Users that install OCP on servers that communicate through transparent
proxies must trust the proxy's CA for the communication to work.

The only way users can currently do that is by using both infraenv ignition overrides and install-config overrides. These are generic messy APIs that are very error prone. We should give users a more formal, simpler API to achieve both at the same time.

The epic surrounding this PR MGMT-11520 aims to resolve this by allowing
the user to specify an "Additional Trust Bundle", in essence the CA for
the proxy.

This work has already been completed for the REST API in https://issues.redhat.com/browse/MGMT-11455

The aim of this PR is to ensure that the same changes are effective in
KubeAPI CRD's
  • Loading branch information
paul-maidment committed Jul 19, 2023
1 parent 7d25541 commit 6fb0883
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 10 deletions.
7 changes: 7 additions & 0 deletions api/v1beta1/infraenv_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ type InfraEnvSpec struct {
// Applicable for both iPXE, and ISO streaming from Image Service.
// +optional
KernelArguments []KernelArgument `json:"kernelArguments,omitempty"`

// PEM-encoded X.509 certificate bundle. Hosts discovered by this
// infra-env will trust the certificates in this bundle. Clusters formed
// from the hosts discovered by this infra-env will also trust the
// certificates in this bundle.
// +optional
AdditionalTrustBundle string `json:"additionalTrustBundle,omitempty"`
}

type KernelArgument struct {
Expand Down
2 changes: 1 addition & 1 deletion cmd/agentbasedinstaller/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func register(ctx context.Context, log *log.Logger, bmInventory *client.Assisted
}

modelsInfraEnv, err := agentbasedinstaller.RegisterInfraEnv(ctx, log, bmInventory, pullSecret,
modelsCluster, RegisterOptions.InfraEnvFile, RegisterOptions.NMStateConfigFile, RegisterOptions.ImageTypeISO)
modelsCluster, RegisterOptions.InfraEnvFile, RegisterOptions.NMStateConfigFile, RegisterOptions.ImageTypeISO, "")
if err != nil {
log.Fatal("Failed to register infraenv with assisted-service: ", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/agentbasedinstaller/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func RegisterCluster(ctx context.Context, log *log.Logger, bmInventory *client.A
}

func RegisterInfraEnv(ctx context.Context, log *log.Logger, bmInventory *client.AssistedInstall, pullSecret string, modelsCluster *models.Cluster,
infraEnvPath string, nmStateConfigPath string, imageTypeISO string) (*models.InfraEnv, error) {
infraEnvPath string, nmStateConfigPath string, imageTypeISO string, additionalTrustBundle string) (*models.InfraEnv, error) {

log.Info("Registering infraenv")

Expand Down
6 changes: 6 additions & 0 deletions config/crd/bases/agent-install.openshift.io_infraenvs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ spec:
items:
type: string
type: array
additionalTrustBundle:
description: PEM-encoded X.509 certificate bundle. Hosts discovered
by this infra-env will trust the certificates in this bundle. Clusters
formed from the hosts discovered by this infra-env will also trust
the certificates in this bundle.
type: string
agentLabels:
additionalProperties:
type: string
Expand Down
6 changes: 6 additions & 0 deletions config/crd/resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2454,6 +2454,12 @@ spec:
items:
type: string
type: array
additionalTrustBundle:
description: PEM-encoded X.509 certificate bundle. Hosts discovered
by this infra-env will trust the certificates in this bundle. Clusters
formed from the hosts discovered by this infra-env will also trust
the certificates in this bundle.
type: string
agentLabels:
additionalProperties:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ spec:
items:
type: string
type: array
additionalTrustBundle:
description: PEM-encoded X.509 certificate bundle. Hosts discovered
by this infra-env will trust the certificates in this bundle. Clusters
formed from the hosts discovered by this infra-env will also trust
the certificates in this bundle.
type: string
agentLabels:
additionalProperties:
type: string
Expand Down
9 changes: 9 additions & 0 deletions internal/bminventory/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -4940,6 +4940,15 @@ func (b *bareMetalInventory) UpdateInfraEnvInternal(ctx context.Context, params
params.InfraEnvUpdateParams.PullSecret = pullSecretBackup
}

// The OpenShift installer validation code for the additional trust bundle
// is buggy and doesn't react well to additional newlines at the end of the
// certs. We need to strip them out to not bother assisted users with this
// quirk.
if params.InfraEnvUpdateParams.AdditionalTrustBundle != nil {
additionalTrustBundle := strings.TrimSpace(*params.InfraEnvUpdateParams.AdditionalTrustBundle)
params.InfraEnvUpdateParams.AdditionalTrustBundle = &additionalTrustBundle
}

if params, err = b.validateAndUpdateInfraEnvParams(ctx, &params); err != nil {
return nil, common.NewApiError(http.StatusBadRequest, err)
}
Expand Down
14 changes: 8 additions & 6 deletions internal/controller/controllers/infraenv_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ func (r *InfraEnvReconciler) updateInfraEnv(ctx context.Context, log logrus.Fiel
if infraEnv.Spec.SSHAuthorizedKey != internalInfraEnv.SSHAuthorizedKey {
updateParams.InfraEnvUpdateParams.SSHAuthorizedKey = &infraEnv.Spec.SSHAuthorizedKey
}
if strings.TrimSpace(infraEnv.Spec.AdditionalTrustBundle) != internalInfraEnv.AdditionalTrustBundle {
updateParams.InfraEnvUpdateParams.AdditionalTrustBundle = &infraEnv.Spec.AdditionalTrustBundle
}

pullSecret, err := r.PullSecretHandler.GetValidPullSecret(ctx, getPullSecretKey(infraEnv.Namespace, infraEnv.Spec.PullSecretRef))
if err != nil {
Expand Down Expand Up @@ -373,16 +376,14 @@ func (r *InfraEnvReconciler) ensureISO(ctx context.Context, log logrus.FieldLogg
clusterID = cluster.ID
openshiftVersion = cluster.OpenshiftVersion
}
infraEnvInternal, err = r.createInfraEnv(ctx, log, &key, infraEnv, clusterID, openshiftVersion)
infraEnvInternal, err = r.createInfraEnv(ctx, log, &key, infraEnv, clusterID, openshiftVersion, infraEnv.Spec.AdditionalTrustBundle)
if err != nil {
log.Errorf("fail to create InfraEnv: %s, ", infraEnv.Name)
return r.handleEnsureISOErrors(ctx, log, infraEnv, err, nil)
} else {
return r.updateInfraEnvStatus(ctx, log, infraEnv, infraEnvInternal)
}
} else {
return r.handleEnsureISOErrors(ctx, log, infraEnv, err, infraEnvInternal)
return r.updateInfraEnvStatus(ctx, log, infraEnv, infraEnvInternal)
}
return r.handleEnsureISOErrors(ctx, log, infraEnv, err, infraEnvInternal)
}

// Check for updates from user and update the infraenv
Expand All @@ -406,6 +407,7 @@ func CreateInfraEnvParams(infraEnv *aiv1beta1.InfraEnv, imageType models.ImageTy
CPUArchitecture: infraEnv.Spec.CpuArchitecture,
ClusterID: clusterID,
OpenshiftVersion: openshiftVersion,
AdditionalTrustBundle: infraEnv.Spec.AdditionalTrustBundle,
},
}
if infraEnv.Spec.Proxy != nil {
Expand All @@ -429,7 +431,7 @@ func CreateInfraEnvParams(infraEnv *aiv1beta1.InfraEnv, imageType models.ImageTy
}

func (r *InfraEnvReconciler) createInfraEnv(ctx context.Context, log logrus.FieldLogger, key *types.NamespacedName,
infraEnv *aiv1beta1.InfraEnv, clusterID *strfmt.UUID, openshiftVersion string) (*common.InfraEnv, error) {
infraEnv *aiv1beta1.InfraEnv, clusterID *strfmt.UUID, openshiftVersion string, additionalTrustBundle string) (*common.InfraEnv, error) {

pullSecret, err := r.PullSecretHandler.GetValidPullSecret(ctx, getPullSecretKey(infraEnv.Namespace, infraEnv.Spec.PullSecretRef))
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions internal/controller/controllers/infraenv_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,7 @@ var _ = Describe("infraEnv reconcile", func() {
Value: "p2",
},
},
AdditionalTrustBundle: "AdditionalTrustBundle",
})
params := CreateInfraEnvParams(infraEnvImage, models.ImageType(imageType), pullSecretString, cluster.ID, cluster.OpenshiftVersion)

Expand All @@ -1114,6 +1115,7 @@ var _ = Describe("infraEnv reconcile", func() {
Expect(params.InfraenvCreateParams.IgnitionConfigOverride).To(Equal(infraEnvImage.Spec.IgnitionConfigOverride))
Expect(params.InfraenvCreateParams.SSHAuthorizedKey).To(Equal(&infraEnvImage.Spec.SSHAuthorizedKey))
Expect(params.InfraenvCreateParams.KernelArguments).To(Equal(internalKernelArgs(infraEnvImage.Spec.KernelArguments)))
Expect(params.InfraenvCreateParams.AdditionalTrustBundle).To(Equal("AdditionalTrustBundle"))
})
})

Expand Down
4 changes: 2 additions & 2 deletions subsystem/agent_based_installer_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("RegisterClusterAndInfraEnv", func() {

modelInfraEnv, registerInfraEnvErr := agentbasedinstaller.RegisterInfraEnv(ctx, log, userBMClient, pullSecret,
modelCluster, "../docs/hive-integration/crds/infraEnv.yaml",
"../docs/hive-integration/crds/nmstate.yaml", "full-iso")
"../docs/hive-integration/crds/nmstate.yaml", "full-iso", "")

Expect(registerInfraEnvErr).NotTo(HaveOccurred())
Expect(*modelInfraEnv.Name).To(Equal("myinfraenv"))
Expand All @@ -59,7 +59,7 @@ var _ = Describe("RegisterClusterAndInfraEnv", func() {

modelInfraEnv, registerInfraEnvErr := agentbasedinstaller.RegisterInfraEnv(ctx, log, userBMClient, pullSecret,
modelCluster, "../docs/hive-integration/crds/infraEnv.yaml",
"../docs/hive-integration/crds/nmstate.yaml", "full-iso")
"../docs/hive-integration/crds/nmstate.yaml", "full-iso", "")

Expect(registerInfraEnvErr).NotTo(HaveOccurred())
Expect(*modelInfraEnv.Name).To(Equal("myinfraenv"))
Expand Down
88 changes: 88 additions & 0 deletions subsystem/kubeapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,70 @@ const (
clusterImageSetName = "openshift-v4.9.0"
)

const additionalTrustCertificate = `-----BEGIN CERTIFICATE-----
MIIFPjCCAyagAwIBAgIUV3ZmDsSwF6/E2CPhFChz3w14OLMwDQYJKoZIhvcNAQEL
BQAwFjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wHhcNMjIxMTI3MjM0MjMwWhcNMzIx
MTI0MjM0MjMwWjAWMRQwEgYDVQQDDAtleGFtcGxlLmNvbTCCAiIwDQYJKoZIhvcN
AQEBBQADggIPADCCAgoCggIBALxURtV3Wd8NEFIplXSZpIdx5I0jFU8thmb2vZON
oNxr31OsHYqkA07RpGSmyn+hv03OI9g4AzuMGs48XoPxZGtWUr0wany1LDDW8t/J
PytYeZyXAJM0zl6/AlzSzYRPk22LykdzVBQosUeRP42a2xWEdDRkJqxxBHQ0eLiC
9g32w57YomhbgCR2OnUxzVmMuQmk987WG7u3/ssSBPEuIebOoX+6G3uLaw/Ka6zQ
XGzRgFq3mskPVfw3exQ46WZfgu6PtG5zxKmty75fNPPwdyw+lwm3u8pH5jpJYvOZ
RHbk7+nxWxLxe5r3FzaNeWskb24J9x53nQzwfcF0MtuRvMycO1i/3e5Y4TanEmmu
GbUOKlJxyaFQaVa2udWAxZ8w1W5u4aKrBprXEAXXDghXbxrgRry2zPO1vqZ/aLH8
YKnHLifjdsNMxrA3nsKAViY0erwYmTF+c551gxkW7vZCtJStzDcMVM16U76jato7
fNb64VUtviVCWeHvh7aTpxENPCh6T8eGh3K4HUESTNpBggs3TXhF1yEcS+aKVJ3z
6CZcke1ph/vpMt/684xx8tICp2KMWbwk3nIBaMw84hrVZyKFgpW/gZOE+ktV91zw
LF1oFn+2F8PwGSphBwhBE0uoyFRNmUXiPsHUyEh7kF7EU5gb1sxTzM5sWCNm6nIS
QRlXAgMBAAGjgYMwgYAwHQYDVR0OBBYEFHuAjvmIDJX76uWtnfirReeBU+f2MB8G
A1UdIwQYMBaAFHuAjvmIDJX76uWtnfirReeBU+f2MA8GA1UdEwEB/wQFMAMBAf8w
LQYDVR0RBCYwJIILZXhhbXBsZS5jb22CD3d3dy5leGFtcGxlLm5ldIcECgAAATAN
BgkqhkiG9w0BAQsFAAOCAgEACn2BTzH89jDBHAy1rREJY8nYhH8GQxsPQn3MZAjA
OiAQRSqqaduYdM+Q6X3V/A8n2vtS1vjs2msQwg6uNN/yNNgdo+Nobj74FmF+kwaf
hodvMJ7z+MyeuxONYL/rbolc8N031nPWim8HTQsS/hxiiwqMHzgz6hQou1OFPwTJ
QdhsfXgqbNRiMkF/UxLfIDEP8J5VAEzVJlyrGUrUOuaMU6TZ+tx1VbNQm3Xum5GW
UgtmE36wWp/M1VeNSsm3GOQRlyWFGmE0sgA95IxLRMgL1mpd8IS3iU6TVZLx0+sA
Bly38R1z8Vcwr1vOurQ8g76Epdet2ZkQNQBwvgeVvnCsoy4CQf2AvDzKgEeTdXMM
WdO6UnG2+PgJ6YQHyfCB34mjPqrJul/0YwWo/p+PxSHRKdJZJTKzZPi1sPuxA2iO
YiJIS94ZRlkPxrD4pYdGiXPigC+0motT6cYxQ8SKTVOs7aEax/xQngrcQPLNXTgn
LtoT4hLCJpP7PTLgL91Dvu/dUMR4SEUNojUBul67D5fIjD0sZvJFZGd78apl/gdf
PxkCHm4A07Zwl/x+89Ia73mk+y8O2u+CGh7oDrO565ADxKj6/UhxhVKmV9DG1ono
AjGUGkvXVVvurf5CwGxpwT/G5UXpSK+314eMVxz5s3yDb2J2J2rvIk6ROPxBK0ws
Sj8=
-----END CERTIFICATE-----`

const additionalTrustCertificate2 = `-----BEGIN CERTIFICATE-----
MIIFPjCCAyagAwIBAgIUBCE1YX2zJ0R/3NURq2XQaciEuVQwDQYJKoZIhvcNAQEL
BQAwFjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wHhcNMjIxMTI3MjM0MjAyWhcNMzIx
MTI0MjM0MjAyWjAWMRQwEgYDVQQDDAtleGFtcGxlLmNvbTCCAiIwDQYJKoZIhvcN
AQEBBQADggIPADCCAgoCggIBAKY589W+Xifs9SfxofBI1r1/NKsMUVPvg3ZtDIPQ
EeNKf5OgtSOVFcoEmkS7ZWNTIu4Kd1WBf/rG+F5lm/aTTa3j720Q+fS+gsveGQPz
7taUpU/TjHHzoCqjjhaYMr4gIJ3jkpTXUWG5/vka/oNykSxkGCuZw1gyXHNujA8L
DJYY8VNUHPl5MmXGaT++6yEN4WdB2f7R/MmEaH6KnGo/LjhMeiVmDsIxHZ/xW9OR
izPklnUi78NfZJSxiknoV6CnQShNijLEq6nQowYQ1lQuNWs6sTM28I0BYWk+gDUz
NOWkVqSHFRMzGmpqYJs7JQiv0g33VN/92dwdP/kZc9sAYRqDaI6hplOZrD/OEsbG
lmN90x/o42wotJeBDN1hHlJ1JeRjR1Vk8XUfOmaTuOPzooKIM0h9K6Ah6u3lRQtE
n68yxn0sGD8yw6EydS5FD9zzvA6rgXBSsvpMFjk/N/FmnIzD4YinLEiflfub1O0M
9thEOX9IaOh00U2eGsRa/MOJcCZ5TUOgxVlv15ATUPHo1MW8QkmYOVx4BoM/Bw0J
0HibIU8VUw2AV1tupRdQma7Qg5gyjdx2doth78IG5+LkX95fSyz60Kf9l1xBQHNA
kVyzkXlx8jmdm53CeFvHVOrVrLuA2Dk+t21TNL1uFGgQ0iLxItCf1O6F6B78QqhI
YLOdAgMBAAGjgYMwgYAwHQYDVR0OBBYEFE6DFh3+wGzA8dOYBTL9Z0CyxLJ/MB8G
A1UdIwQYMBaAFE6DFh3+wGzA8dOYBTL9Z0CyxLJ/MA8GA1UdEwEB/wQFMAMBAf8w
LQYDVR0RBCYwJIILZXhhbXBsZS5jb22CD3d3dy5leGFtcGxlLm5ldIcECgAAATAN
BgkqhkiG9w0BAQsFAAOCAgEAoj+elkYHrek6DoqOvEFZZtRp6bPvof61/VJ3kP7x
HZXp5yVxvGOHt61YRziGLpsFbuiDczk0V61ZdozHUOtZ0sWB4VeyO1pAjfd/JwDI
CK6olfkSO78WFQfdG4lNoSM9dQJyEIEZ1sbvuUL3RHDBd9oEKue+vsstlM9ahdoq
fpTTFq4ENGCAIDvaqKIlpjKsAMrsTO47CKPVh2HUpugfVGKeBRsW1KAXFoC2INS5
7BY3h60jFFW6bz0v+FnzW96Mt2VNW+i/REX6fBaR4m/QfG81rA2EEmhxCGrany+N
6DUkwiJxcqBMH9jA2yVnF7BgwG2C3geBqXTTlvVQJD8GOktkvgLjlHcYqO1pI7B3
wP9F9ZF+w39jXwGMGBg8+/aQz1RjP2bOb18n7d0bc4/pbbkVAmE4sq4qMneFZAVE
uj9S2Jna3ut08ZP05Ych5vCGX4VJ8gNNgrJju2PJVBl8NNyDfHKeHfWSOR9uOMjT
vqK6iRD9xqu/oLJyrlAuOL8ZxRpeqjxF/g8NYYV/fvv8apaX58ua9qYAFQVGf590
mmjOozzn9VBqKenVmfwzen5v78CBSgS4Hd72Qp42rLCNgqI8gyQa2qZzaNjLP/wI
pBpFC21fkybGYPkislPQ3EI69ZGRafWDBjlFFTS3YkDM98tqTZD+JG4STY+ivHhK
gmY=
-----END CERTIFICATE-----`

var (
imageSetsData = map[string]string{
"openshift-v4.9.0": "quay.io/openshift-release-dev/ocp-release:4.9.11-x86_64",
Expand Down Expand Up @@ -2257,6 +2321,30 @@ var _ = Describe("[kube-api]cluster installation", func() {
}, "2m", "10s").Should(Equal(0))
})

It("Should populate AdditionalTrustBundle on creation and update of infraenv", func() {
infraEnvKey := types.NamespacedName{
Namespace: Options.Namespace,
Name: infraNsName.Name,
}
deployInfraEnvCRD(ctx, kubeClient, infraNsName.Name, &v1beta1.InfraEnvSpec{
PullSecretRef: secretRef,
SSHAuthorizedKey: sshPublicKey,
AdditionalTrustBundle: additionalTrustCertificate,
})
Eventually(func() string {
return getInfraEnvFromDBByKubeKey(ctx, db, infraEnvKey, waitForReconcileTimeout).AdditionalTrustBundle
}, "1m", "10s").Should(Equal(additionalTrustCertificate))

By("Update infraenv accepts AdditionalTrustBundle")
infraEnvCrd := getInfraEnvCRD(ctx, kubeClient, infraEnvKey)
infraEnvCrd.Spec.AdditionalTrustBundle = additionalTrustCertificate2
err := kubeClient.Update(ctx, infraEnvCrd)
Expect(err).To(BeNil())
Eventually(func() string {
return getInfraEnvFromDBByKubeKey(ctx, db, infraEnvKey, waitForReconcileTimeout).AdditionalTrustBundle
}, "1m", "10s").Should(Equal(additionalTrustCertificate2))
})

It("deploy clusterDeployment and infraEnv and verify cluster updates", func() {
deployClusterDeploymentCRD(ctx, kubeClient, clusterDeploymentSpec)
deployAgentClusterInstallCRD(ctx, kubeClient, aciSpec, clusterDeploymentSpec.ClusterInstallRef.Name)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 6fb0883

Please sign in to comment.