coop-cloud/abra
coop-cloud
/
abra
5
2
Fork 15
Code Issues Pull Requests 4 Packages Releases 36 Activity

test: dns unit test & ensure catalogue present #360

Merged
decentral1se merged 3 commits from domain-checks-improved into main 2023-09-30 06:29:50 +00:00
Conversation 3 Commits 3 Files Changed 6 +54 -7
decentral1se commented 2023-09-29 11:27:54 +00:00
Owner
Copy Link

See coop-cloud/organising#495. RE: what was mentioned in coop-cloud/organising#495 (comment), I can't implement a linting rule because we want to do a comparison with the app config and not just on the recipe config. The linting only focuses on the recipe config atm. Hopefully the improved error messages & new check to match app domain root / server domain root help here. See #360 (comment)

@cas @3wordchant

~~See https://git.coopcloud.tech/coop-cloud/organising/issues/495. RE: what was mentioned in https://git.coopcloud.tech/coop-cloud/organising/issues/495#issuecomment-18008, I can't implement a linting rule because we want to do a comparison with the app config and not just on the recipe config. The linting only focuses on the recipe config atm. Hopefully the improved error messages & new check to match app domain root / server domain root help here.~~ See https://git.coopcloud.tech/coop-cloud/abra/pulls/360#issuecomment-18025 @cas @3wordchant
decentral1se added 1 commit 2023-09-29 11:27:55 +00:00
continuous-integration/drone/push Build is failing Details
continuous-integration/drone/pr Build is failing Details
b2a3d70ed3
fix: ensure app domain root matches server domain root 
See coop-cloud/organising#495
decentral1se added 1 commit 2023-09-29 11:52:59 +00:00
continuous-integration/drone/push Build is failing Details
continuous-integration/drone/pr Build is failing Details
c6e2fd99ef
test: ensure catalogue is present for testing 
See https://build.coopcloud.tech/coop-cloud/abra/1504/1/4
decentral1se force-pushed domain-checks-improved from c6e2fd99ef to 9d437f935e 2023-09-29 11:53:57 +00:00 Compare
decentral1se force-pushed domain-checks-improved from 9d437f935e to cb467c1f91 2023-09-29 12:02:01 +00:00 Compare
3wordchant commented 2023-09-29 13:29:15 +00:00
Owner
Copy Link

@decentral1se not sure i fully understand the changes, so apologies if this is barking up the wrong tree... but what about an app with the domain foo.com hosted on swarm.bar.com? Shouldn't that still be supported?

@decentral1se not sure i fully understand the changes, so apologies if this is barking up the wrong tree... but what about an app with the domain `foo.com` hosted on `swarm.bar.com`? Shouldn't that still be supported?
3wordchant commented 2023-09-29 13:31:46 +00:00
Owner
Copy Link

if i'm reading it right, for the current example of docs.coopcloud.tech being hosted on swarm.autonomic.zone (didn't get to that part of the migration yet 😬), domainRoot = coopcloud.tech, serverRoot = autonomic.zone, so we'll see"root of app domain (coopcloud.tech) does not match root of server domain (autonomic.zone)?"?

if i'm reading it right, for the current example of `docs.coopcloud.tech` being hosted on `swarm.autonomic.zone` (didn't get to that part of the migration yet 😬), `domainRoot` = `coopcloud.tech`, `serverRoot` = `autonomic.zone`, so we'll see`"root of app domain (coopcloud.tech) does not match root of server domain (autonomic.zone)?"`?
decentral1se commented 2023-09-30 06:12:12 +00:00
Author
Owner
Copy Link

@3wordchant tysm for reviewing! I'm pretty happy to see that the tests are a nice way to grok the changes ☺️ Yeh, you're right, this change is broken. Looking at the code again, I guess the existing check in 906bf65d47/pkg/dns/dns.go (L44-L47) is what we want. So, I will drop the new changes and try keep the tests.

@3wordchant tysm for reviewing! I'm pretty happy to see that the tests are a nice way to grok the changes ☺️ Yeh, you're right, this change is broken. Looking at the code again, I guess the existing check in https://git.coopcloud.tech/coop-cloud/abra/src/commit/906bf65d475ea7295ea3e27958a2b76c9bb1bde2/pkg/dns/dns.go#L44-L47 is what we want. So, I will drop the new changes and try keep the tests.
decentral1se force-pushed domain-checks-improved from cb467c1f91 to f18f0b6f82 2023-09-30 06:26:49 +00:00 Compare
decentral1se changed title from fix: ensure app domain root matches server domain root to test: dns unit test & ensure catalogue present 2023-09-30 06:28:41 +00:00
decentral1se merged commit f18f0b6f82 into main 2023-09-30 06:29:50 +00:00
decentral1se deleted branch domain-checks-improved 2023-09-30 06:29:50 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something is not working

  
build

go build related issues

  
ci/cd

Building things with CI/CD

  
contributing

Dealing with contributing

  
design

UI/UX

  
documentation

Documenting all the things

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
invalid

Something is wrong

  
meta

relating to the project or repo itself

  
question

More information is needed

  
security

Security related

  
wontfix

This won't be fixed

No Label
bug
build
ci/cd
contributing
design
documentation
duplicate
enhancement
help wanted
invalid
meta
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: coop-cloud/abra#360
No description provided.
Delete Branch "domain-checks-improved"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?