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.

Sign up for GitHub

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

Jump to bottom

NodeJS API Upgrade #5044

Closed
binarymist opened this issue Oct 10, 2018 · 26 comments
Closed

NodeJS API Upgrade #5044

binarymist opened this issue Oct 10, 2018 · 26 comments
Assignees
@binarymist
Labels
API Client Issues in the ZAP API clients. historic Type-Task

Comments

@binarymist
Copy link
Contributor

Working on this now:
https://github.com/zaproxy/zaproxy/blob/develop/src/org/zaproxy/zap/extension/api/NodeJSAPIGenerator.java
https://www.npmjs.com/package/zaproxy
@psiinon
Copy link
Member

psiinon commented Oct 10, 2018

Great :)
FYI we'd like to move the NodeJS API to https://github.com/zaproxy/zap-api-nodejs :)
The NodeJSAPIGenerator can stay where it is though.

@thc202 thc202 mentioned this issue Oct 10, 2018
Closed
@binarymist
Copy link
Contributor Author

binarymist commented Oct 10, 2018 •
edited
Loading

Yip, changes (first round) have already been made in source locally. Just need to run the generator.

@binarymist
Copy link
Contributor Author

binarymist commented Oct 29, 2018 •
edited
Loading

Can we please enable issues on zap-api-nodejs?

@binarymist
Copy link
Contributor Author

Is it intentional that we're using a more restrictive license on zap-api-nodejs than zaproxy ?

@kingthorin
Copy link
Member

https://github.com/zaproxy/zap-api-nodejs/blob/master/LICENSE
https://github.com/zaproxy/zaproxy/blob/develop/LICENSE

Both Apache License 2.0???

@binarymist
Copy link
Contributor Author

Yes, those two are @kingthorin , but the License in the link I provided above is MIT

@thc202
Copy link
Member

thc202 commented Oct 29, 2018

Issues are not enabled since we manage them here, in the main repo. That's the same for all API clients.

@thc202
Copy link
Member

thc202 commented Oct 29, 2018

I don't know why MIT was chosen.

@kingthorin
Copy link
Member

Yes, those two are @kingthorin , but the License in the link I provided above is MIT

Weird, maybe if you dig through the history/blame there's something about it...

@psiinon
Copy link
Member

psiinon commented Oct 29, 2018

If its a rewrite we can change the license now? I'd prefer us to stick with Apache v2 unless we have to do with something else...

This was referenced Oct 29, 2018
Merged
Merged
@binarymist
Copy link
Contributor Author

binarymist commented Oct 30, 2018 •
edited by thc202
Loading

  • Todo: @binarymist:

  • Todo: @psiinon / @thc202:

    • Run the alternative generaters to create the rest of the files -> commit -> push, or show @binarymist how to do this
    • Add the zap-api-nodejs repo to snyk to show there are zero vulnerabilities in the package
    • Tag and npm publish according to semver. Let me know when you get close to this, as it may pay to do a rc as in 1.0.0-rc.1

This was referenced Oct 31, 2018
Merged
Merged
Merged
Merged
thc202 added a commit to thc202/zap-extensions that referenced this issue Nov 8, 2018
@thc202
Update API generator for 2.7.0 and 2.8.0
cc5d73e 
Move ApiGenerator to api package to access core class.
Add Websocket API to ApiGenerator.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.
Update generate-apis in build.xml file to include just the required
files on the classpath and to depend on compile to work by itself.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
thc202 added a commit to thc202/zap-extensions that referenced this issue Nov 8, 2018
@thc202
Add build target to generate the APIs (beta)
4405e2d 
Add target generate-apis to build.xml file.
Move ApiGenerator to api package to access core class.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
thc202 added a commit to thc202/zap-extensions that referenced this issue Nov 8, 2018
@thc202
Add build target to generate the APIs (alpha)
256b26a 
Add target generate-apis to build.xml file.
Move ApiGenerator to api package to access core class.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
This was referenced Nov 8, 2018
Merged
Merged
Merged
psiinon pushed a commit to zaproxy/zap-extensions that referenced this issue Nov 9, 2018
@thc202 @psiinon
Add build target to generate the APIs (alpha) (#1886)
410a0f9 
Add target generate-apis to build.xml file.
Move ApiGenerator to api package to access core class.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
psiinon pushed a commit to zaproxy/zap-extensions that referenced this issue Nov 9, 2018
@thc202 @psiinon
Add build target to generate the APIs (beta) (#1885)
8794b82 
Add target generate-apis to build.xml file.
Move ApiGenerator to api package to access core class.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
@thc202
Copy link
Member

thc202 commented Nov 13, 2018

The repo has been added to snyk.

@binarymist
Copy link
Contributor Author

Can you please add the snyk badge to the README @thc202 ?

@thc202
Copy link
Member

thc202 commented Nov 13, 2018

Sure.

@binarymist
Copy link
Contributor Author

Then I guess:

  1. npm version 1.0.0-rc.1
  2. npm publish
  3. Update CHANGELOG.md

@psiinon
Copy link
Member

psiinon commented Nov 14, 2018

  1. Add to https://lgtm.com/ (done)
  2. Publicise via ZAP groups, twitter etc :)

@psiinon
Copy link
Member

psiinon commented Nov 14, 2018

lgtm has analysed the code now. Looks good until you show the files it excludes: https://lgtm.com/projects/g/zaproxy/zap-api-nodejs/latest/files/?sort=name&dir=ASC&mode=heatmap&showExcluded=true
158 issues but hopefully most of those have the same underlying cause

@thc202
Copy link
Member

thc202 commented Nov 14, 2018

Yes, all of them have the same cause, @binarymist already planned addressing that (zaproxy/zap-api-nodejs#1 (comment)).

@binarymist
Copy link
Contributor Author

Are we ready for publish?

@psiinon
Copy link
Member

psiinon commented Nov 15, 2018

If you're happy with it them I am :)

@binarymist
Copy link
Contributor Author

Are you doing it @psiinon / @thc202 ? If not I'm happy to if you can supply creds.

@thc202
Copy link
Member

thc202 commented Nov 15, 2018

We can do the release, but we should look into adding permissions to you as well.

I've a question regarding the release steps, wouldn't that leave the tag with an "incomplete" changelog, it would still say "unreleased" instead of the tagged version?

@thc202
Copy link
Member

thc202 commented Nov 16, 2018

The release has been done.

@thc202 thc202 added the API Client Issues in the ZAP API clients. label Nov 16, 2018
@thc202
Copy link
Member

thc202 commented Nov 16, 2018

@binarymist do you want the issue open for the follow up changes or can it be closed?

@binarymist
Copy link
Contributor Author

We can close if you like, I'll create another issue with the known changes.

@psiinon psiinon closed this as completed Nov 17, 2018
This was referenced Nov 21, 2018
Closed
Closed
Closed
Closed
Open
Open
adioss pushed a commit to adioss/zap-extensions that referenced this issue Dec 11, 2018
@thc202 @adioss
Update API generator for 2.7.0 and 2.8.0
0dc1249 
Move ApiGenerator to api package to access core class.
Add Websocket API to ApiGenerator.
Change ApiGenerator to make use of the new core method (>=2.8.0) which
allows to pass a ResourceBundle, to include the descriptions of the API
endpoints being generated.
Add dummy Messages.properties to be able to work with ZAP 2.7.0.
Update generate-apis in build.xml file to include just the required
files on the classpath and to depend on compile to work by itself.

For zaproxy/zaproxy#5044 - NodeJS API Upgrade
@lock
Copy link

lock bot commented Feb 3, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Feb 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@binarymist binarymist

Labels
API Client Issues in the ZAP API clients. historic Type-Task
Milestone
No milestone
Development

No branches or pull requests
4 participants
@psiinon @binarymist @thc202 @kingthorin