Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix association with custom cert without CA #5314

Merged
merged 5 commits into from
Feb 16, 2022

Conversation

thbkrkr
Copy link
Contributor

@thbkrkr thbkrkr commented Feb 1, 2022

Follow-up of #5277 to adress the case of a custom cert without CA (issued by a well-known certificate authority).

In this case, the CA is configured but the CA cert is not provided, which is translated in the AssocConf stored in an annotation of the associated resource to CASecretName defined and CACertProvided set to false. I did not rechallenge this logic.

caCertProvided := len(expectedSecret.Data[certificates.CAFileName]) > 0
return CASecret{Name: expectedSecret.Name, CACertProvided: caCertProvided}, nil

The main fix is to replace the use of CAIsConfigured by GetCACertProvided when we setup a CA config setting.
This also fixes Agent->Fleet where we forgot to check that CAIsConfigured() to setup the CA env var (discovered in #5240 (review))
And finally, I noticed that we could reuse WriteAssocsToConfigHash which gives the advantage of taking into account the authSecret content in the annotated config hash.

Changes:

  • 8fb1935 Setup CA config setting if GetCACertProvided instead of CAIsConfigured for KB|ENT|EMS->x associations
  • eb11a77 Setup FleetCA env var for Agent only if GetCACertProvided
  • 60c480a Update WriteAssocsToConfigHash to use GetCACertProvided instead of CAIsConfigured
  • 695ad4c Reuse WriteAssocsToConfigHash for APM|ENT|EMS->x associations

@thbkrkr
Copy link
Contributor Author

thbkrkr commented Feb 8, 2022

I missed the initial intention. We mount the CA secret using a volume in the pods when the secret exists (works when we mount the secret as a directory without directly targetting the CA file) but we don't configure the CA in the configurations when the CA is not provided in the secret. This means we can keep the calls to CAIsConfigured when it's about configuring the pod without referencing ca.crt.

Verified

This commit was signed with the committer’s verified signature.
blakeembrey Blake Embrey
…ured` for KB|ENT|EMS->x associations
@thbkrkr thbkrkr force-pushed the fix-assoc-with-custom-cert-without-ca branch from 23169d8 to 695ad4c Compare February 9, 2022 10:08
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am OK with merging this given we follow up in #5379

@thbkrkr thbkrkr merged commit f9c1a4b into elastic:main Feb 16, 2022
@thbkrkr thbkrkr added the v2.1.0 label Feb 16, 2022
@thbkrkr thbkrkr deleted the fix-assoc-with-custom-cert-without-ca branch March 22, 2022 16:36
fantapsody pushed a commit to fantapsody/cloud-on-k8s that referenced this pull request Feb 7, 2023
* Setup CA config setting if `GetCACertProvided` instead of `CAIsConfigured` for KB|ENT|EMS->x associations
* Setup FleetCA env var for Agent only if `GetCACertProvided`
* Update `WriteAssocsToConfigHash` to use `GetCACertProvided` instead of `CAIsConfigured`
* Reuse `WriteAssocsToConfigHash` for APM|ENT|EMS->x associations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants