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

feat: Pebble Notices #20

Merged
merged 13 commits into from
Jun 7, 2024
Merged

feat: Pebble Notices #20

merged 13 commits into from
Jun 7, 2024

Conversation

kayra1
Copy link
Contributor

@kayra1 kayra1 commented Jun 4, 2024

Description

This change introduces pebble notices to the GoCert's backend as included in spec TE034. It is enabled in the configuration file. Each update to a CSR's related certificate will create a new pebble notice, which will allow the charm to listen to changes in GoCert and make relevant changes to the charm relations.

The test for this feature is inside of build_rock.yaml. This is both because testing the execution of external commands is difficult without mocking, and to make sure that there is no doubt that pebble is actually receiving the request in a regular rock deployment.

$ docker exec gocert /usr/bin/pebble notices

ID   User    Type           Key  First               Repeated            Occurrences
1    public  change-update  1    today at 07:32 UTC  today at 07:32 UTC  4
2    public  change-update  2    today at 07:32 UTC  today at 07:32 UTC  3

$ <submit CSR through curl>
stdout:

% Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0     0 --:--:-- --:--:-- --:--:--     0
100  1082  100     1  100  1081    147  155k --:--:-- --:--:-- --:--:--  176k

stderr: 1

$ <submit certificate through curl>

stdout:

% Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  1334  100     1  100  1333     99   129k --:--:-- --:--:-- --:--:--  144k

stderr: 1

$ docker exec gocert /usr/bin/pebble notices

1ID   User    Type           Key                            First               Repeated            Occurrences
1    public  change-update  1                              today at 07:32 UTC  today at 07:32 UTC  4
2    public  change-update  2                              today at 07:32 UTC  today at 07:32 UTC  3
3    0       custom         gocert.com/certificate/create  today at 07:32 UTC  today at 07:32 UTC  1

When the pebble binary is missing or any other error is produced, an error is logged, but if the change was successfully committed to the database, the GoCert application does not roll back the changes and responds to the HTTP request regularly. This is to support deployments outside of the Rock or any custom containers without pebble installed. It's also not preferable for users to go through the effort of uploading a cert, get everything right, then get rejected for something that's out of their control.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that validate the behaviour of the software
  • I validated that new and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@kayra1 kayra1 force-pushed the TLSENG-239-pebble-notices branch from 6a277f6 to eff68d6 Compare June 5, 2024 16:06
@kayra1 kayra1 marked this pull request as ready for review June 5, 2024 16:30
@kayra1 kayra1 requested a review from a team as a code owner June 5, 2024 16:30
Copy link
Contributor

@ghislainbourgeois ghislainbourgeois left a comment

Choose a reason for hiding this comment

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

2 small nitpicks, otherwise LGTM

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
kayra1 and others added 2 commits June 5, 2024 20:15
Co-authored-by: Ghislain Bourgeois <[email protected]>
Co-authored-by: Ghislain Bourgeois <[email protected]>
internal/api/server.go Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@kayra1 kayra1 requested a review from gruyaume June 6, 2024 08:58
internal/api/handlers.go Outdated Show resolved Hide resolved
@saltiyazan
Copy link
Contributor

When the pebble binary is missing or any other error is produced, an error is logged, but if the change was successfully committed to the database, the GoCert application does not roll back the changes and responds to the HTTP request regularly. This is to support deployments outside of the Rock or any custom containers without pebble installed. It's also not preferable for users to go through the effort of uploading a cert, get everything right, then get rejected for something that's out of their control.

What if there was an error and we couldn't send a pebble notification for this specific certificate to the charm? would the charm still be able to get it later somehow? or will the charm miss that change?

@kayra1
Copy link
Contributor Author

kayra1 commented Jun 6, 2024

When the pebble binary is missing or any other error is produced, an error is logged, but if the change was successfully committed to the database, the GoCert application does not roll back the changes and responds to the HTTP request regularly. This is to support deployments outside of the Rock or any custom containers without pebble installed. It's also not preferable for users to go through the effort of uploading a cert, get everything right, then get rejected for something that's out of their control.

What if there was an error and we couldn't send a pebble notification for this specific certificate to the charm? would the charm still be able to get it later somehow? or will the charm miss that change?

Any error that may occur with pebble is simply out of our control, we provide a rock with pebble guaranteed to be installed. So if any error were to occur, the cause would be pebble/charm specific or container image specific.

If you mean intermittent runtime errors, pebble notifications not working would mean pebble is broken. This would break a lot of things with charms, like replans and restarts or any container interactions.

In this case GoCert notices are just as reliable as the infrastructure that supports it, and there's not much we could do about that.

So yes, long story short charms will miss that event.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
kayra1 and others added 2 commits June 6, 2024 15:16
Co-authored-by: Guillaume Belanger <[email protected]>
Co-authored-by: Guillaume Belanger <[email protected]>
@kayra1 kayra1 enabled auto-merge (squash) June 7, 2024 07:44
@kayra1 kayra1 merged commit 2142819 into main Jun 7, 2024
11 checks passed
@kayra1 kayra1 deleted the TLSENG-239-pebble-notices branch June 7, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants