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: allow generator to pull from private npm repo #1061

Merged
merged 69 commits into from
Jan 18, 2024

Conversation

AayushSaini101
Copy link
Contributor

@AayushSaini101 AayushSaini101 commented Nov 7, 2023

Resolves #538
Resolves #1100

Description:

  • Allow the Generator to pull the template from the private repo. [ Check docs/installing-privateTemplate.md]
  • Adding new testing to verify the Generator pulls template from the private Repository.
  • Integrate project tests now run in isolation, each separately
  • Now tests do not make any changes to the file system of the host (entrypoint script) - they run longer but yeah, there is no other way to do it
  • The code of html-template no longer needs to be stored inside the test project
  • Some await/async bugs and removal of redundant promises
  • update arborist to as latest as possible without breaking node 12 support

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@AayushSaini101
Copy link
Contributor Author

@derberg Hello Sir. I updated the PR according to your instructions. Can you please review :)

@derberg
Copy link
Member

derberg commented Nov 22, 2023

sorry, few days delay from my side, not feeling well

@aeworxet fyi ☝🏼

@AayushSaini101
Copy link
Contributor Author

sorry, few days delay from my side, not feeling well

@aeworxet fyi ☝🏼

@magicmatatjahu @jonaslagoni Can you please review this

@derberg
Copy link
Member

derberg commented Dec 12, 2023

Hey there, sorry a lot for delay.

There are some improvements we need here:

  • please solve merge conflicts
  • sonarcloud shows 3 code smells - please address them
  • in documentation it is not enough to just explain how to configure to use another NPM registry, we should have an example there on how to do it. I think we need a new, separate markdown file that explains things
  • there is a huge amount of code changes that do not seem like related to the goal of this PR which makes review super hard. I'm not sure if they are automated by some of the tools you use or what. Please make sure PR focus on the topic it addresses and do not refactor irrelevant parts
  • generator now uses https://github.com/smoya/multi-parser-js which contains all custom schema parsers. And you see in code https://github.com/asyncapi/generator/blob/master/lib/generator.js#L32. So why do you add all the parsers to generator.js again?

@AayushSaini101
Copy link
Contributor Author

Thanks for the suggestions working on them

@AayushSaini101
Copy link
Contributor Author

Thanks for the suggestions working on them

@derberg I have done some changes how can i test it locally i checked the documentation but cannot able to find the suitable method how to test the changes locally can you tell me some points . So that i can ensure to test before pushing

@AayushSaini101
Copy link
Contributor Author

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues 0 Security Hotspots No data about Coverage 0.0% Duplication on New Code

See analysis details on SonarCloud

Working on fix it

@AayushSaini101
Copy link
Contributor Author

@derberg Done, Do we need to add documentation in generator or update in AsyncapiDoc

@AayushSaini101
Copy link
Contributor Author

Thanks working on fix

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

added few comments

also please see why tests are failing

last but not least please specify how did you test new functionality to make sure it works - like integration test I mean

lib/generator.js Outdated Show resolved Hide resolved
docs/template.md Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
@AayushSaini101
Copy link
Contributor Author

added few comments

also please see why tests are failing

last but not least please specify how did you test new functionality to make sure it works - like integration test I mean

Fix Test cases and added new test cases:

image

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

left some comments

I'm wondering how we could test it to make sure private registry works. Setting up automated test will be huge, but maybe we could at least have a short instruction for manual testing whenever we modify this functionality 🤔

so basically:

you know, like developer guide

test/generator.test.js Outdated Show resolved Hide resolved
test/generator.test.js Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
@AayushSaini101
Copy link
Contributor Author

left some comments

I'm wondering how we could test it to make sure private registry works. Setting up automated test will be huge, but maybe we could at least have a short instruction for manual testing whenever we modify this functionality 🤔

so basically:

you know, like developer guide

Added small doc @derberg

@aeworxet
Copy link

@asyncapi/bounty_team

@AayushSaini101
Copy link
Contributor Author

left some comments
I'm wondering how we could test it to make sure private registry works. Setting up automated test will be huge, but maybe we could at least have a short instruction for manual testing whenever we modify this functionality 🤔
so basically:

you know, like developer guide

Added small doc @derberg

@derberg I used verdaccio to create private repository in the local.

@AayushSaini101
Copy link
Contributor Author

AayushSaini101 commented Dec 21, 2023

@derberg Can you please check before you go for holidays 🙏🏼

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

Sorry for not reviewing before holidays, I just had to many topics to close

lib/generator.js Outdated Show resolved Hide resolved

**Step 1:** Settuping Private Repository in Local:

Installing [verdaccio](https://verdaccio.org/) using below command:
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking, wouldn't it be better to hide all of this and just extend https://github.com/asyncapi/generator/tree/master/test/test-project that is responsible for testing generator in isolated environment 🤔

If I'm correct you would just have to:

so with above you are getting rid of step 1-4 from this instruction

and to get rid last step:

Thoughts? this way we get one document less to maintain, and out of the box good quality integration test that would work out of the box as we have GitHub Action workflow for it -> https://github.com/asyncapi/generator/blob/master/.github/workflows/pr-testing-with-test-project.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking, wouldn't it be better to hide all of this and just extend https://github.com/asyncapi/generator/tree/master/test/test-project that is responsible for testing generator in isolated environment 🤔

If I'm correct you would just have to:

so with above you are getting rid of step 1-4 from this instruction

and to get rid last step:

Thoughts? this way we get one document less to maintain, and out of the box good quality integration test that would work out of the box as we have GitHub Action workflow for it -> https://github.com/asyncapi/generator/blob/master/.github/workflows/pr-testing-with-test-project.yml

@derberg I have tried using docker but we have to manually publish the template the repository, after providing the username and password in config, we have to still login, i have tried https://github.com/verdaccio/verdaccio/tree/master/docker-examples/v4/https-portal-example this example but also we have to manually publish the repo, Event the official video is publishing the repo manually [ We have to manually ]login https://www.youtube.com/watch?v=zRI0skF1f8I&t=234s&ab_channel=Docker
image

  • Also i cannot pass parameters in the cli for login and publishing the repository.
image

I guess existing the small doc is sufficient to help developers

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

Please push to repo what you have in regards to verdaccio so I can try to help on my side. But first I need to see what configs you already tried

lib/generator.js Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
lib/generator.js Outdated Show resolved Hide resolved
lib/generator.js Show resolved Hide resolved
@AayushSaini101
Copy link
Contributor Author

Please push to repo what you have in regards to verdaccio so I can try to help on my side. But first I need to see what configs you already tried

Sure @derberg

  • Extend https://github.com/asyncapi/generator/blob/master/test/test-project/docker-compose.yml to handle startup of the verdaccio registry [Done]
  • Startup the verdaccio and config the password i created new because mentioned one is not working. [Done]
  • Add package name in the conf that is html-template and the default package**[Done]**
  • I have to provide login and password credentials and packages are not present after building the services.
    Screenshot 2024-01-04 at 5 13 42 PM 1

@AayushSaini101
Copy link
Contributor Author

Sonar Cloud issue can be avoided because it is suggested to use optional changing that will cause lint issue

title: "Using private templates"
weight: 180
---
[Generator](https://www.asyncapi.com/tools/generator) allows to fetch the template from the private repositories like verdaccio, nexus, npm etc. Let's understand how can use the Generator to fetch the templates from the private repositories.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[Generator](https://www.asyncapi.com/tools/generator) allows to fetch the template from the private repositories like verdaccio, nexus, npm etc. Let's understand how can use the Generator to fetch the templates from the private repositories.
Generator allows to fetch the template from the private repositories like verdaccio, nexus, npm etc.

no need to have one more sentence + link to generator - as long these are generator docs 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg


* **token:** Optional parameter to pass npm registry auth token that you can grab from .npmrc file

## Example to pull Private template:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Example to pull Private template:
## Pulling private template using library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg

Comment on lines 20 to 35
const generator = new Generator('@asyncapi/html-template', outputDir,
{
debug: true,
install: true,
forceWrite: true,
templateParams: {
singleFile: true
},
registry: {
url: 'http://verdaccio:4873',
auth: 'YWRtaW46bmltZGE='
// base64 encoded username and password
// represented as admin:nimda

}
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const generator = new Generator('@asyncapi/html-template', outputDir,
{
debug: true,
install: true,
forceWrite: true,
templateParams: {
singleFile: true
},
registry: {
url: 'http://verdaccio:4873',
auth: 'YWRtaW46bmltZGE='
// base64 encoded username and password
// represented as admin:nimda
}
});
const generator = new Generator('@asyncapi/html-template', 'output',
{
debug: true,
registry: {
url: 'http://verdaccio:4873',
auth: 'YWRtaW46bmltZGE='
// base64 encoded username and password
// represented as admin:nimda
}
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg

}
});
```
Let's suppose the template name as @asyncapi/html-template is present in the private repository, In order to pull the template from the private repository, we need to pass the url and auth as a parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Let's suppose the template name as @asyncapi/html-template is present in the private repository, In order to pull the template from the private repository, we need to pass the url and auth as a parameters.
Let's assume you host `@asyncapi/html-template` in private package registry like Verdaccio. In order to pull this template you need to provide `registry.url` option that points to the registry URL and `registry.auth` as base64 encoded value that represents username and password. Instead of username and password you can also pass `registry.token`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg

[Generator](https://www.asyncapi.com/tools/generator) allows to fetch the template from the private repositories like verdaccio, nexus, npm etc. Let's understand how can use the Generator to fetch the templates from the private repositories.


## Parameters that needs to pass in Generator to pull private template:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Parameters that needs to pass in Generator to pull private template:
## Private registry options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg

Comment on lines 10 to 15
* **URL:** The URL of the registry where is private template is present.
* **Auth:** Optional parameter to pass npm registry username and password encoded with base64, formatted like username:password value should be encoded.

**For example**: if the username and password is admin and nimda, we need to base64 encoded of admin:nimda.

* **token:** Optional parameter to pass npm registry auth token that you can grab from .npmrc file
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* **URL:** The URL of the registry where is private template is present.
* **Auth:** Optional parameter to pass npm registry username and password encoded with base64, formatted like username:password value should be encoded.
**For example**: if the username and password is admin and nimda, we need to base64 encoded of admin:nimda.
* **token:** Optional parameter to pass npm registry auth token that you can grab from .npmrc file
* **registry.url**: The URL of the registry where is private template is present. Defaults to `registry.npmjs.org`
* **registry.auth**: Optional parameter to pass npm registry username and password encoded with base64, formatted like `username:password`. For example if the username and password is `admin` and `nimda`, you need to encode with base64 value like `admin:nimda` which results in `YWRtaW46bmltZGE=`.
* **registry.token** : Optional parameter to pass to npm registry auth token. To get the token you can first authenticate with registry using `npm login` and then grab generated token from `.npmrc` file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done @derberg

derberg
derberg previously approved these changes Jan 17, 2024
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

Well done 👏🏼
That was a big task, something that I definitely did not expect will be so advanced

but as a result we have lots of bugs solved, much better testing setup and very good reliable integration tests

@derberg
Copy link
Member

derberg commented Jan 17, 2024

@Florence-Njeri this PR has some docs, please have a look and approve 🙏🏼

Copy link
Collaborator

@Florence-Njeri Florence-Njeri left a comment

Choose a reason for hiding this comment

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

Editorial review

docs/using-private-template.md Outdated Show resolved Hide resolved
docs/using-private-template.md Outdated Show resolved Hide resolved
docs/using-private-template.md Outdated Show resolved Hide resolved
docs/using-private-template.md Outdated Show resolved Hide resolved
docs/using-private-template.md Outdated Show resolved Hide resolved
docs/using-private-template.md Outdated Show resolved Hide resolved
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@derberg
Copy link
Member

derberg commented Jan 18, 2024

/rtm

@asyncapi-bot asyncapi-bot merged commit b5ff633 into asyncapi:master Jan 18, 2024
14 checks passed
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty AsyncAPI Bounty program related label ready-to-merge released
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

Bug: Need to perform exclusive testing in generator Generator doesn't pull template from private repo
6 participants