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

Revert "feat (codespaces) - Add initial support for codespaces (#27483)" #27655

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

ajcvickers
Copy link
Member

This reverts commit 9599197.

Having a password checked in to efcore/devcontainer.json is not allowed.

/cc @mmitche @dougbu @wtgodbe @Pilchie @ShreyasJejurkar

@mmitche
Copy link
Member

mmitche commented Mar 16, 2022

Note that this won't unblock the mirroring for this branch, that will have to be done separately.

@ajcvickers
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ShreyasJejurkar
Copy link
Contributor

May I please know what is the problem with the devcontainer.json file?
If I understood correctly, keeping the password in connection string is not good is it?

That database is just for running local EF tests, not critical data I guess!

@ajcvickers
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ajcvickers
Copy link
Member Author

@ShreyasJejurkar I don't think there is anything wrong with it, but it isn't allowed. If there isn't any other way to do it, and we can demonstrate that there is no risk, then we may be able to get an exception to the rule. I am reverting this until we find out more.

@ShreyasJejurkar
Copy link
Contributor

As of now, I am not aware of any other method. And also somewhere we need to pass that password right. Maybe in the connection string or env variable. But no matter where we store, we need to tell that in the devcontainer.json file to instruct codespaces.

@ajcvickers
Copy link
Member Author

@ShreyasJejurkar I agree that this sucks. I don't agree with the rules. I don't think there is really a problem here. But I don't make the rules.

@ShreyasJejurkar
Copy link
Contributor

I just saw this codespaces secret, but not sure will that works for this or not. https://docs.github.com/en/codespaces/managing-your-codespaces/managing-encrypted-secrets-for-your-codespaces

@ShreyasJejurkar
Copy link
Contributor

@ShreyasJejurkar I agree that this sucks. I don't agree with the rules. I don't think there is really a problem here. But I don't make the rules.

Yes, @ajcvickers I can totally understand. Programming is easy, organizations are hard.
I can understand from an MS point of view as well that it's not good practice to store passwords in human-readable format, but also there isn't any other way I found! :(

@ajcvickers
Copy link
Member Author

@ShreyasJejurkar Which may mean we can't enable codespaces.

@ShreyasJejurkar
Copy link
Contributor

ShreyasJejurkar commented Mar 17, 2022

@ShreyasJejurkar Which may mean we can't enable codespaces.

"As of now". (I hope)

Sorry for the pain. For sure I will research a little bit on this how we can avoid storing passwords in devcontainer.

I will look forward to safia @captainsafia response on this as well, to know if she knew any way to workaround on this.

@ajcvickers ajcvickers merged commit e0a4b88 into main Mar 18, 2022
@ajcvickers ajcvickers deleted the Puddles0316 branch March 18, 2022 09:54
@captainsafia
Copy link
Member

Codespaces do support managing secrets per this doc. However, someone with admin access to the settings page will have to add the local test password to the secret on the page.

If this is done @ShreyasJejurkar, is the SQL testing extension able to access secrets from an environment variable instead of a literal string?

@ShreyasJejurkar
Copy link
Contributor

@captainsafia I am yet to try this, just yesterday I found that.

Not sure will that work or not correctly, but looks like a huge work for the new-ish contributor to EF Core.

@ShreyasJejurkar
Copy link
Contributor

https://code.visualstudio.com/docs/remote/devcontainerjson-reference
Searching for secretes in the reference file, it looks like we can grab them in postcontainer or precontainer scripts file. But I guess we need to httprequest to GitHub API to grab them in bash (Looks like a bit of work just to get secret). And then we can set that env variable to connection string with that secret value which is db password, so that the tests can utilize it to run db dependent tests.

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