Skip to content

ROX-9129: Determine secret expiration by parsing certificates stored in secrets#471

Merged
juanrh merged 14 commits intomasterfrom
juanrh/ROX-9129
Feb 8, 2022
Merged

ROX-9129: Determine secret expiration by parsing certificates stored in secrets#471
juanrh merged 14 commits intomasterfrom
juanrh/ROX-9129

Conversation

@juanrh
Copy link
Contributor

@juanrh juanrh commented Jan 28, 2022

Description

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • Evaluated and added CHANGELOG entry if required
  • Determined and documented upgrade steps

No CHANGELOG entry or upgrade steps required.

Testing Performed

Added unit tests. Test are passing run as go test -count=100 -race -v -p=1 github.com/stackrox/rox/sensor/kubernetes/localscanner -run TestGetSecretRenewalTime

@ghost
Copy link

ghost commented Jan 28, 2022

Tag for build #189113 is 3.68.x-174-g223a2eab7f.

💻 For deploying this image using the dev scripts, run the following first:

export MAIN_IMAGE_TAG='3.68.x-174-g223a2eab7f'

📦 You can also generate an installation bundle with:

docker run -i --rm stackrox/main:3.68.x-174-g223a2eab7f central generate interactive > bundle.zip

🕹️ A roxctl binary artifact can be downloaded from CircleCI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, doesn't this code always return the first certificate found instead of returning the shorter expiration date?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renewalTimeInitialized is set to true the first time the renewalTime is set, so subsequent updates to renewalTime are only done if secretRenewalTime.Before(renewalTime). In any case I've added a test for GetSecretsCertRenewalTime that also checks this

Copy link
Contributor

@porridge porridge left a comment

Choose a reason for hiding this comment

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

Just a general note that it would be great to keep this code rather generic (i.e. not assume the secret is for scanner, since this would also be useful for https://issues.redhat.com/browse/ROX-8131

@juanrh
Copy link
Contributor Author

juanrh commented Feb 1, 2022

Just a general note that it would be great to keep this code rather generic (i.e. not assume the secret is for scanner, since this would also be useful for https://issues.redhat.com/browse/ROX-8131

I removed the references to local scanner in sensor/kubernetes/localscanner/certificate_expiration.go

@juanrh
Copy link
Contributor Author

juanrh commented Feb 2, 2022

Turning into draft again until the repository interface in #457 is determined, as that would affect this change too

@juanrh juanrh marked this pull request as ready for review February 3, 2022 17:34
@juanrh juanrh merged commit f0c4fde into master Feb 8, 2022
@juanrh juanrh deleted the juanrh/ROX-9129 branch February 8, 2022 12:58
RTann pushed a commit that referenced this pull request Apr 6, 2022
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.

3 participants