Merge pull request #489 from kubeshop/f1ames/fix/git-url-parsing

fix(synchronizer): improve git url parsing
kubeshop · Aug 23, 2023 · bf8798a · bf8798a
2 parents a9c4dac + 9b6ba48
commit bf8798a
Show file tree

Hide file tree

Showing 8 changed files with 170 additions and 28 deletions.
diff --git a/.changeset/nasty-fireants-double.md b/.changeset/nasty-fireants-double.md
@@ -0,0 +1,5 @@
+---
+"@monokle/synchronizer": patch
+---
+
+Fixed invalid git URL parsing
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/synchronizer/README.md b/packages/synchronizer/README.md
@@ -99,7 +99,7 @@ const policy = await authenticator.getPolicy('/home/kubeshope/...');
 
 // By repo data
 const policy = await authenticator.getPolicy({
-  provider: 'github',
+  provider: 'github.com',
   remote: 'origin',
   owner: 'kubeshop',
   name: 'monokle-core',

diff --git a/packages/synchronizer/package.json b/packages/synchronizer/package.json
@@ -41,15 +41,18 @@
   "dependencies": {
     "@monokle/validation": "*",
     "env-paths": "^3.0.0",
+    "git-url-parse": "^13.1.0",
     "mkdirp": "^3.0.1",
     "node-fetch": "^3.3.2",
     "normalize-url": "^8.0.0",
     "openid-client": "^5.4.3",
     "simple-git": "^3.19.1",
+    "slugify": "^1.6.6",
     "yaml": "^2.3.1"
   },
   "devDependencies": {
     "@types/chai": "^4.3.5",
+    "@types/git-url-parse": "^9.0.1",
     "@types/mocha": "^10.0.1",
     "@types/sinon": "^10.0.16",
     "c8": "^8.0.1",

diff --git a/.../github-kubeshop-monokle-core.policy.yaml → ...thubcom-kubeshop-monokle-core.policy.yaml b/.../github-kubeshop-monokle-core.policy.yaml → ...thubcom-kubeshop-monokle-core.policy.yaml
diff --git a/packages/synchronizer/src/__tests__/synchronizer.spec.ts b/packages/synchronizer/src/__tests__/synchronizer.spec.ts
@@ -42,7 +42,7 @@ describe('Synchronizer Tests', () => {
       const synchronizer = createDefaultMonokleSynchronizer(new StorageHandlerPolicy(storagePath));
 
       const policy = await synchronizer.getPolicy({
-        provider: 'github',
+        provider: 'github.com',
         remote: 'origin',
         owner: 'kubeshop',
         name: 'monokle-core',
@@ -55,24 +55,24 @@ describe('Synchronizer Tests', () => {
     });
 
     it('returns policy if there is policy file (from path)', async () => {
-      const storagePath = await createTmpConfigDir('github-kubeshop-monokle-core.policy.yaml');
+      const storagePath = await createTmpConfigDir('githubcom-kubeshop-monokle-core.policy.yaml');
       const synchronizer = createDefaultMonokleSynchronizer(new StorageHandlerPolicy(storagePath));
 
       const policy = await synchronizer.getPolicy(storagePath);
 
       assert.isObject(policy);
       assert.isTrue(policy.valid);
       assert.isNotEmpty(policy.path);
-      assert.match(policy.path, /github-kubeshop-monokle-core.policy.yaml$/);
+      assert.match(policy.path, /githubcom-kubeshop-monokle-core.policy.yaml$/);
       assert.isNotEmpty(policy.policy);
     });
 
     it('returns policy if there is policy file (from git data)', async () => {
-      const storagePath = await createTmpConfigDir('github-kubeshop-monokle-core.policy.yaml');
+      const storagePath = await createTmpConfigDir('githubcom-kubeshop-monokle-core.policy.yaml');
       const synchronizer = createDefaultMonokleSynchronizer(new StorageHandlerPolicy(storagePath));
 
       const policy = await synchronizer.getPolicy({
-        provider: 'github',
+        provider: 'github.com',
         remote: 'origin',
         owner: 'kubeshop',
         name: 'monokle-core',
@@ -81,7 +81,7 @@ describe('Synchronizer Tests', () => {
       assert.isObject(policy);
       assert.isTrue(policy.valid);
       assert.isNotEmpty(policy.path);
-      assert.match(policy.path, /github-kubeshop-monokle-core.policy.yaml$/);
+      assert.match(policy.path, /githubcom-kubeshop-monokle-core.policy.yaml$/);
       assert.isNotEmpty(policy.policy);
       assert.isNotEmpty(policy.policy.plugins);
     });
@@ -172,7 +172,7 @@ describe('Synchronizer Tests', () => {
       stubs.push(queryApiStub);
 
       const repoData = {
-        provider: 'github',
+        provider: 'github.com',
         remote: 'origin',
         owner: 'kubeshop',
         name: 'monokle-core',
@@ -187,7 +187,7 @@ describe('Synchronizer Tests', () => {
       assert.isObject(newPolicy);
       assert.isTrue(newPolicy.valid);
       assert.isNotEmpty(newPolicy.path);
-      assert.match(newPolicy.path, /github-kubeshop-monokle-core.policy.yaml$/);
+      assert.match(newPolicy.path, /githubcom-kubeshop-monokle-core.policy.yaml$/);
       assert.isNotEmpty(newPolicy.policy);
       assert.isNotEmpty(newPolicy.policy.plugins);
       assert.isNotEmpty(newPolicy.policy.rules);
@@ -271,7 +271,7 @@ describe('Synchronizer Tests', () => {
       stubs.push(queryApiStub);
 
       const repoData = {
-        provider: 'github',
+        provider: 'github.com',
         remote: 'origin',
         owner: 'kubeshop',
         name: 'monokle-core',
@@ -283,7 +283,7 @@ describe('Synchronizer Tests', () => {
             assert.isObject(policy);
             assert.isTrue(policy.valid);
             assert.isNotEmpty(policy.path);
-            assert.match(policy.path, /github-kubeshop-monokle-core.policy.yaml$/);
+            assert.match(policy.path, /githubcom-kubeshop-monokle-core.policy.yaml$/);
             assert.isNotEmpty(policy.policy);
             assert.isNotEmpty(policy.policy.plugins);
             assert.isNotEmpty(policy.policy.rules);

diff --git a/packages/synchronizer/src/handlers/gitHandler.ts b/packages/synchronizer/src/handlers/gitHandler.ts
@@ -1,3 +1,4 @@
+import gitUrlParse from 'git-url-parse';
 import {simpleGit} from 'simple-git';
 import type {RemoteWithRefs} from 'simple-git';
 
@@ -20,18 +21,18 @@ export class GitHandler {
     }
 
     const url = remote.refs.push;
-    // With generic git support in Cloud, this should also become generic. The same for 'provider' field.
-    const match = url.match(/github\.com(\/|:)([^\/]+)\/([^\/]+)\.git/);
-    if (!match) {
+    try {
+      const urlParts = gitUrlParse(url);
+
+      return {
+        provider: urlParts.source,
+        remote: remote.name,
+        owner: urlParts.owner,
+        name: urlParts.name,
+      };
+    } catch (err: any) {
       return undefined;
     }
-
-    return {
-      provider: 'github',
-      remote: remote.name,
-      owner: match[2],
-      name: match[3],
-    };
   }
 
   async isGitRepo(folderPath: string) {