Skip to content

docs: Add note on constraints for defaultPath#8658

Merged
vikram-dagger merged 3 commits into
dagger:mainfrom
vikram-dagger:docs-351-default-context-constraints
Nov 5, 2024
Merged

docs: Add note on constraints for defaultPath#8658
vikram-dagger merged 3 commits into
dagger:mainfrom
vikram-dagger:docs-351-default-context-constraints

Conversation

@vikram-dagger
Copy link
Copy Markdown
Contributor

No description provided.

@vikram-dagger vikram-dagger requested a review from a team as a code owner October 9, 2024 13:03
Comment thread docs/current_docs/manuals/developer/functions.mdx Outdated
- resolution always starts from the directory containing a `dagger.json` file.

:::important
For security reasons, it is not possible to retrieve files or directories outside the default context's root directory.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is conflating a few terms. To clarify:

  • default path: a path within the context directory, to load a directory/file as a default value
  • context directory: repo root if in a git repo, or the root directory otherwise
  • root directory: the directory where dagger.json is
  • source directory: the sub-directory within the root directory, used by the SDK (source field in dagger.json)

So there's no "default context". This is more correct:

Suggested change
For security reasons, it is not possible to retrieve files or directories outside the default context's root directory.
For security reasons, it is not possible to retrieve files or directories outside the context directory.

But the terminology may be a leaking implementation detail if there's a simpler way to explain this (without conflicting with current terminology).

Copy link
Copy Markdown
Contributor Author

@vikram-dagger vikram-dagger Oct 15, 2024

Choose a reason for hiding this comment

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

I've modified this entire section using the terms "context directory" and "default path". I've not mentioned "source directory" as it did not seem relevant to this section, but let me know in case I got that wrong. I've also updated the example table. Let me know if this version is clearer.

@vikram-dagger vikram-dagger force-pushed the docs-351-default-context-constraints branch from 5a3f174 to e2bb6ae Compare October 18, 2024 18:24
Copy link
Copy Markdown
Member

@TomChv TomChv left a comment

Choose a reason for hiding this comment

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

Left only 1 comment, otherwise it's much more clear! Thanks for these changes

| `.` | `dagger call -m my-module read-dir --source=public` | `./public` |
| `..` | `dagger call -m my-module read-dir --source=public` | `./public` |

:::tip
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should link that to #8739

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@vikram-dagger vikram-dagger force-pushed the docs-351-default-context-constraints branch from e2bb6ae to d81b9d8 Compare October 24, 2024 16:50
Comment thread docs/current_docs/api/arguments.mdx Outdated
Copy link
Copy Markdown
Member

@TomChv TomChv left a comment

Choose a reason for hiding this comment

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

Answered the question, please fix the table ;)

Comment thread docs/current_docs/api/arguments.mdx Outdated
Vikram Vaswani added 2 commits November 5, 2024 14:52
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
@vikram-dagger vikram-dagger force-pushed the docs-351-default-context-constraints branch from d81b9d8 to 43d7359 Compare November 5, 2024 09:22
Signed-off-by: Vikram Vaswani <vikram@dagger.io>
@vikram-dagger vikram-dagger merged commit b5d2c4f into dagger:main Nov 5, 2024
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 this pull request may close these issues.

4 participants