Git Product home page Git Product logo

Comments (35)

mikehaertl avatar mikehaertl commented on June 6, 2024

Yes, I think that makes sense. As long as it's just composer - and not any global plugins :). We should leave the latter decision to the developer.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

🙇 😉

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

See 5af9e95

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

We also need the asset-plugin, since Yii 2.0 requires it.

Otherwise you can't even do: composer create-project yiisoft/yii2

from yii2-docker.

and800 avatar and800 commented on June 6, 2024

IMO either composer or asset plugin should not be in the minimal image. This is not a part of application, just a build tool. When we deploy the app in production, we don't run composer install on production server, instead we build the image (extend our base image) with source code embedded in it (including vendor dir), and then transfer this image to production server.

So, if you aim to create reusable image for both dev and prod, composer is quite an arguable component of it.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

We also need the asset-plugin, since Yii 2.0 requires it.

I don't think we really need it: There's asset-packagist which works fine. And many of us simply don't need any bower packages (apart from those that come with yii2). So we should leave that decision to the developer. The big down point with fxp asset-plugin is, that it's so obtrusive: Once installed, you have no choice but use it (or better: wait for it...). If we really include it, it must be made absolutely sure, that it's only activated on demand and not by default.

@and800 I'd say composer makes sense: It's very small and if you don't need it you don't have to use it. But if you need it, it's there and you don't need it locally installed or from another docker image.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

More reasons why we should include composer in any image:

  • This is a yii2 base image and yii2 is heavily based on the composer ecosystem. So you could say that what docker-php-ext-install is for the php docker image is composer for our yii2 image. It's part of the build process and the image should help you with that.
  • Without composer there's almost no real value to the -min image. Those that really want a "pure" min image can also build it from scratch. It's only 2 extensions - and they probably have to add their DB driver anyway.

Someone also brought up security concerns when having composer in the image. I don't share this opinion. If an attacker is able to execute code on your container then you have a much bigger problem. As an attacker I would surely not focus on executing composer but rather some plain shell commands.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

I don't think we really need it: There's asset-packagist which works fine. And many of us simply don't need any bower packages (apart from those that come with yii2).

It's currently a Yii 2.0 requirement - asset-packagist has also it's issues.

The big down point with fxp asset-plugin is, that it's so obtrusive:

It's your personal dislike against the asset-plugin speaking here, many of it's issues are solved since a while.

My position on this is clear:

min

  • no composer

common

  • composer with asset-plugin
    • can be disabled/enabled via config from 1.3 on which is very likely to be released before a stable version of these docker images

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

It's your personal dislike against the asset-plugin speaking here, many of it's issues are solved since a while.

I believe it when I see it.

I think you're trying to make -min / -common something different. They really only exist because of different PHP extensions in the first place. Not because of differences in included tools. There we should always only include the minimum - but I think that composer is part of that minimum.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

I don't really see it as PHP-extensions only.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

Well, that was the initial idea. I must know, because I create the issue #7 for it ;).

from yii2-docker.

and800 avatar and800 commented on June 6, 2024

@schmunk42 Could you please state what is wrong with asset-packagist and why it cannot be suitable choice instead of asset-plugin?

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

@and800 Please see yiisoft/yii2#8688 (comment) for details.

It can be a suitable choice, but you can also run into issues, i.e. if you're requesting a new package for the first time.

The very best option would be to use asset-packagist and fxp as a fallback - but that's another issue.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

The very best option would be to use asset-packagist and fxp as a fallback - but that's another issue.

IMO the best choice is to let the developer decide. Sounds fair to me. No one says fxp plugin is bad. But still not everyone needs it, so it just should be less obtrusive as it is now.

But we're getting OT. Back to composer:

The main problem I have with composer is the github API key issue. I've used an ugly workaround in our codemix/yii2-dockerbase image, that allows you to pass the API key as env var:

https://github.com/codemix/yii2-dockerbase/blob/master/2.0/apache/composer

But this is dangerous, as it could encourage people to add their API key at build time. In that case it would become part of the image and that's a no-go. I just can't see a better way how to handle this.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

The main problem I have with composer is the github API key issue.

Isn't that a development only issue?

Building images with packages from a composer.lock file (yiisoft/yii2-app-basic#112) should not require an API key. Only if you have none or run composer update, which should not be done during build anyway.

In fact, if you run just install from a lock-file in the build process, you do not even need fxp ... I just checked this, take an application template with a lock file, like phd ;) And install with a vanilla composer with Docker

docker run -it -v ${PWD}:/app composer install --ignore-platform-reqs

But for sure we also wanna run updates in the container. But this also implies a large number of other things to take care about, like caching, host-volume performance, etc...

Maybe it makes sense to classify the tools we need a bit more (also related to: #7)

composer

  • build (-common?)
  • update (-development)

fxp

  • update (-development)

openssh, zlib (git)

  • build (as a fallback and ie. when using private repos)
  • update (-development)

mysql, intl, .... (libs)

  • runtime (-min)
  • update (-development)

npm

  • build (-full?)
  • update (-development)

That's what I meant when saying min/common/full does not only affect runtime extensions. The requirements or expectations are "multidimensional" or let's say -common is a very loose description. But it does also not make sense to build 4 or 5 flavors for every image.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

Isn't that a development only issue?

Yes, but these images here are meant for both, production and development. That's one of the main selling points for a docker workflow. The same image should be used everywhere and we don't need another -development flavour. We only have to make sure, that we provide a way to make composer update work.

Not sure, why you bring up npm again - that's something for a js project. You have dedicated containers for that. We should focus on composer in this issue here.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

Yes, but these images here are meant for both, production and development. That's one of the main selling points for a docker workflow. The same image should be used everywhere and we don't need another -development flavour. We only have to make sure, that we provide a way to make composer update work.

The workflows are the same, but the images itself are not required to be (that's also related to extends -common from -min), think about Xdebug.

As a valid use-case, take Docker Security scans for example. If you wanna run a production-ready image in a commercial environment like Docker Store you have to pass their scans. This will get harder for every piece you add, if composer (or a required lib) has an open CVE, those would fail and the image would be useless for that purpose.

Not sure, why you bring up npm again - that's something for a js project. You have dedicated containers for that. We should focus on composer in this issue here.

First, it was an example for a possible -full container.
But, it's exactly the same issue you have with composer, there's also a dedicated container for that.
Because if you wanna build (& bundle) everything in Docker build you might have to include additional tools.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

As a valid use-case, take Docker Security scans for example. If you wanna run a production-ready image in a commercial environment like Docker Store you have to pass their scans.

I would say, that's beyond the scope of our images. Applications who have these kind of requirements should definitely build their own base image from scratch as they surely need additional extensions anyway. It makes no difference if they have to add the 3 extensions we have in -min to their Dockerfile.

That's why I still think, that if we drop composer we also don't need any -min as it doesn't offer any real value - it's too trivial then.

Also let's focus on composer here and keep discussion about -full in #7.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

I would say, that's beyond the scope of our images.

Clearly a goal from my side. Would be feasible with an absolutely minimalistic image. Also a benchmark and nothing wrong with it, this can make a huge difference when it comes enterprise grade commercial support and distribution. I'd say that is in Yii's scope.

That's why I still think, that if we drop composer we also don't need any -min as it doesn't offer any real value - it's too trivial then.

It's a runtime for Yii 2.0 framework, that's not trivial at all, see ICU and gdlib issues.


Taking all things into account I'd vote for leaving it out of -min, but having also a script docker-composer-install ready (in -min), which is taking care of the correct installation and can be used in a Dockerfile when building a custom image.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

Clearly a goal from my side.

Then we need to automate this during the test. This will also help to see, if composer is problematic from a security perspective, which I very much doubt.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

if composer is problematic from a security perspective

Might also be a dependency of composer, here's an example screenshot how the scans look like:

bildschirmfoto 2017-03-17 um 15 17 22

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

Let's do an actual scan. Better than guessing what could be.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

@schmunk42 to summarize what we discussed in private, here's what I understand. You suggest that instead of adding composer directly, we provide easy to use installer scripts like:

  • docker-composer-install installs composer + dependencies (git, openssh-client, ...) globally without any plugins
  • docker-composer-install-fxp like above but also installs fxp-asset-plugin

Note: I don't think we'd need docker-composer-install-asset-packagist as there's nothing to install here. It's something that a developer has to configure in his composer.json file instead.

Did I get this right? If so, this solution is maybe really something to consider.

The obvious drawback is that we lose the ability to run composer from the image right away. Instead you have these options:

  • Run docker-composer-install && composer ... each time you want to use it. This will take a long time to run so is probably only feasible for quickl testing something
  • Extend our image in your own project specific base image and run the composer installer in your Dockerfile there. To me that's the most likely scenario anyway.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

Note: I don't think we'd need docker-composer-install-asset-packagist as there's nothing to install here. It's something that a developer has to configure in his composer.json file instead.
Did I get this right? If so, this solution is maybe really something to consider.

Currently more meant as an example, yes. But there might be things coming in the future like support for an AP API AuthToken or whatever (there was a short discussion about a trigger from the asset-plugin ... other story).
Or the installation script might also take care about GitHub API token handling in a centralized place.

The obvious drawback is that we lose the ability to run composer from the image right away.

For now I'd keep the vanilla composer installation in -min, because as far as I can see it, it does not cause a security issue at the moment and because you see it as a requirement ;)

Sidenote: If it's just about composer, you can also run docker run composer, if you need it for debugging.

But still, when I build a custom (development) image from -min, I'd start like so:

FROM yiisoft/yii2-php
RUN docker-composer-install-fxp  \
    &&  docker-php-ext-install cubrid_pdo
COPY . /app
RUN composer install

Building a production image could (depending on your workflow) start like so:

FROM yiisoft/yii2-php
RUN docker-php-ext-install cubrid_pdo
COPY vendor /app/vendor
COPY src /app/src

Above setup would not even need composer

There are various other combinations which might fit better to your workflow. With simple scripts those images can be build very easily.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

For now I'd keep the vanilla composer installation in -min

As already said elsewhere, images should not differ in this regard. So either we have composer everywhere or nowhere.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

As already said elsewhere, images should not differ in this regard. So either we have composer everywhere or nowhere.

Can we agree, that we don't yet agree in this point :)

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

Then I misunderstood your last comment on this in #7:

That's why I still think, tooling for all images should be the same to make them coherent.

I absolutely agree.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

We agree that it should be coherent - maybe coherent will be not to install it by default (we not agree here yet), but to provide a script, with which you can choose to install fxp or not (we agree about the tooling).

Finally convinced to hit the button? I fear to reach my GitHub FormPost limit ;)

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

I can't follow you anymore. Above I said, that you suggest that we may consider not to install composer. You disagreed and said it's only -min where it should always be installed. Then I said: That's not good, it should be the same everywhere. Now you say you agree to this. I'm sorry, but that makes no sense. You contradict yourself.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

You disagreed and said it's only -min where it should always be installed.

No, I - personally - would not install it in -min, but I would keep it, because you said a PHP image without composer would be practically useless. And I don't wanna talk in circles between these two points.

That's not good, it should be the same everywhere. Now you say you agree to this.

I agree to that: everywhere we should have the same ways of installing the tools: docker-composer-install or add additional tools docker-composer-install fxp.

If composer is installed in -min - I really do not care at the moment.

We could continue with: Do we need docker-composer-install git openssl, because - you don't need it - if ... .lock etc...etc...

You contradict yourself

No, I am respecting your concerns about installing composer, while still having a flexible way to react on the outcome of other issues, that's all I am talking about.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

No, I - personally - would not install it in -min, but ...

All this has already been mentioned or discussed in this or the other thread. I tried to sumarize your view in my comment above. A simple "Yes, that's mostly correct" as response would have been enough. Bringing up the same points over and over again doesn't help. So the last 7 comments or so only add more noise to this discussion which make it discouraging for anyone else to participate.

I'm not sure how to proceed.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

We're mixing discussions among different issues and PRs; because they belong together, but anyhow.

Let's settle this one: Install composer in all current images. OK.

from yii2-docker.

mikehaertl avatar mikehaertl commented on June 6, 2024

But that's not what I said. My comment here was an attemt to find a settlement:

#4 (comment)

Quote:

Did I get this right? If so, this solution is maybe really something to consider.

I've also added what I see as drawbacks and how to workaround. But you obviously missed all of that.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

Maybe we have some misunderstandings ...

Note: I don't think we'd need docker-composer-install-asset-packagist as there's nothing to install here. It's something that a developer has to configure in his composer.json file instead.

Did I get this right? If so, this solution is maybe really something to consider.

I thought your question referenced to the "Note:" paragraph, that's why I answered to this.

The rest

docker-composer-install installs composer + dependencies (git, openssh-client, ...) globally without any plugins
docker-composer-install-fxp like above but also installs fxp-asset-plugin

is fine.

Run docker-composer-install && composer ... each time you want to use it. This will take a long time to run so is probably only feasible for quickl testing something

Yes, that's OK for quick testing.

Extend our image in your own project specific base image and run the composer installer in your Dockerfile there. To me that's the most likely scenario anyway.

Yes, 100%.

I thought that was obvious.

You then came up with #4 (comment) - so I also said OK to RUN docker-composer-install in -min.

from yii2-docker.

schmunk42 avatar schmunk42 commented on June 6, 2024

Composer is installed on the base-image, fxpio/cap is not.

from yii2-docker.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.