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

Discussion: Ready for Use #8

Closed
wellwelwel opened this issue Apr 14, 2024 · 5 comments Β· Fixed by #10
Closed

Discussion: Ready for Use #8

wellwelwel opened this issue Apr 14, 2024 · 5 comments Β· Fixed by #10
Milestone

Comments

@wellwelwel
Copy link
Collaborator

wellwelwel commented Apr 14, 2024

Hey @sidorares and @dougwilson πŸ™‹πŸ»β€β™‚οΈ

Current Status

From that point on, any change would be a new idea, so this is where I stop. The aim is to close this issue after the first version has been published on npmjs.

The current state can be seen as a dismemberment of this responsibility.


Discussion

My thought would be to unify the old and new certificates by default, to ensure that current users can continue using the 'Amazon RDS' option without errors for both mysqljs and node-mysql2. Then separate the certificates by period, so new users can use only the certificates they're interested in.

In practice:

  • The default import would bring in all the certificates (preventing major changes).
  • We would update the documentation for the new usage based on this dependency.
  • Next years, when there is a new update from AWS, it will be much easier to include the new certificates.

Topics from #5 (comment):

  • [...] I think the intention was to add later more profiles from other known services.
    • Apart the dependency name, there is no reason to restrict a certificate set from other instances apart from AWS.
  • Are the any other AWS services that have different cert bundle?
  • Are there any other popular services that might benefit from a npm packaged cert distribution?
    • Maybe node-postgres and any Node.js driver for SQL compatible with RDS instances.

An example of how to use this dependency with other SQL drivers for Node.js:

const awsCaBundle = require('aws-ssl-profiles');

const ca = awsCaBundle.ca.join('');

Tests

  • Each certificate is parsed and checked for signatures.
  • Ensure that the array is correctly sized according to the number of certificates.

These tests ensure that the certificates aren't corrupted individually or that the array isn't properly populated.


Notes

Although the current certificates are based on the latest MySQL2 update, this dependency intended to work with both mysqljs and node-mysql2.

As a dependency that can be installed directly with mysqljs and node-mysql2, including other drivers, I opted to bring in all the common documents from the open-source community.

@sidorares
Copy link
Member

@wellwelwel do you want to continue with publishing? I'm not sure what are the optimal steps here, possible option:

  • you push first version from your local computer with your npm credentials
  • optionally: add myself and Doug to the list of npm owners
  • configure release-please secret in this repo with your token

@wellwelwel
Copy link
Collaborator Author

wellwelwel commented Apr 17, 2024

Thank you for your trust πŸ’™

Can I generate a NPM_TOKEN exclusive for this repository and send it to you?


These are the available settings:

Screenshot 2024-04-17 at 11 11 00

@wellwelwel wellwelwel linked a pull request Apr 17, 2024 that will close this issue
@sidorares
Copy link
Member

Can I generate a NPM_TOKEN exclusive for this repository and send it to you?

I'm not sure, previously I'd use npm to generate the token, and it was scoped per account or package, yo can probably do the same ( token scoped to https://www.npmjs.com/package/aws-ssl-profiles package ) and add it to this repo as NPM_TOKEN secret. Probably no need to share it with me, I prefer release-please to publish changes for me but maybe add me as a co-owner to npm as a backup

@wellwelwel
Copy link
Collaborator Author

wellwelwel commented Apr 18, 2024

@sidorares, sorry, I meant that I don't have the privileges to include secrets to this repository. The sharing reason would be for you to include the token as a secret, so we could reactivate the release-please πŸ™‹πŸ»β€β™‚οΈ

Also, the package is published on npm and I invited you and @dougwilson.

@sidorares
Copy link
Member

sidorares commented Apr 18, 2024

@wellwelwel try again, I think I accidentally made you a maintainer, now you are an admin

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 a pull request may close this issue.

2 participants